Skip to content

Commit 5efe089

Browse files
fix(signals): await object thenables and surface their sync rejections (closes #2764, #2765)
Introduce a shared object-only `isThenable` (Promises/A+ shape) used by both the async runtime and `action()`: - handleAsync now captures a synchronously-rejecting thenable and settles it through the same status path an async rejection uses, so the error reaches `Errored` instead of leaving the node stuck on the pending path (#2764). - action() awaits any yielded object thenable, not just `instanceof Promise`, matching `await` semantics for custom/cross-realm promises (#2765). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent d7e382a commit 5efe089

6 files changed

Lines changed: 95 additions & 12 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
`action()` now awaits yielded object thenables, not just native `Promise` instances. Yielding a Promise-like object that is not `instanceof Promise` (a custom thenable, cache wrapper, or cross-realm promise) previously resumed the generator immediately with the raw object instead of its settled value. Yield handling now uses an object-thenability check (`typeof value === "object" && typeof value.then === "function"`), shared with the async runtime's thenable detection (#2765).
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Surface synchronously-rejecting thenables. A memo returning a Promise-like thenable that invoked its rejection handler synchronously during `.then()` (e.g. a cache that already knows it failed) had its error dropped and stayed stuck on the pending path forever — `<Loading>` never gave way to `<Errored>`. The thenable branch now captures a synchronous rejection (mirroring the existing sync-resolve handling) and settles it, so the error reaches the boundary the same way an async rejection does (#2764).

packages/solid-signals/src/core/action.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
setActiveTransition,
88
type Transition
99
} from "./scheduler.js";
10+
import { isThenable } from "./async.js";
1011

1112
function restoreTransition<T>(transition: Transition, fn: () => T): T {
1213
globalQueue.initTransition(transition);
@@ -75,14 +76,14 @@ export function action<Args extends any[], Y, R>(
7576
} catch (e) {
7677
return done(undefined, e);
7778
}
78-
if (r instanceof Promise)
79+
if (isThenable(r))
7980
return void r.then(run, e => restoreTransition(ctx, () => step(e, true)));
8081
run(r);
8182
};
8283

8384
const run = (r: IteratorResult<Y, R>) => {
8485
if (r.done) return done(r.value);
85-
if (r.value instanceof Promise)
86+
if (isThenable(r.value))
8687
return void r.value.then(
8788
v => restoreTransition(ctx, () => step(v)),
8889
e => restoreTransition(ctx, () => step(e, true))

packages/solid-signals/src/core/async.ts

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,21 +138,30 @@ export function settlePendingSource(el: Computed<any>): void {
138138
if (scheduled) schedule();
139139
}
140140

141+
// Object-thenable detection (Promises/A+ shape).
142+
export function isThenable<T>(value: T | PromiseLike<T>): value is PromiseLike<T> {
143+
return (
144+
value != null &&
145+
typeof value === "object" &&
146+
typeof (value as { then?: unknown }).then === "function"
147+
);
148+
}
149+
141150
export function handleAsync<T>(
142151
el: Computed<T>,
143152
result: T | PromiseLike<T> | AsyncIterable<T>,
144153
setter?: (value: T) => void
145154
): T {
146155
let iterator: any = false;
147-
let isThenable = false;
156+
let thenable = false;
148157
if (typeof result === "object" && result !== null) {
149158
untrack(() => {
150159
iterator = (result as any)[Symbol.asyncIterator];
151-
isThenable = !iterator && typeof (result as any).then === "function";
160+
thenable = !iterator && isThenable(result as T | PromiseLike<T>);
152161
});
153162
}
154163

155-
if (!isThenable && !iterator) {
164+
if (!thenable && !iterator) {
156165
el._inFlight = null;
157166
return result as T;
158167
}
@@ -166,7 +175,7 @@ export function handleAsync<T>(
166175
if (__DEV__ && el._config & CONFIG_SYNC) {
167176
const message =
168177
`[SYNC_NODE_RECEIVED_ASYNC] A computed/effect created with \`sync: true\` returned ` +
169-
`${isThenable ? "a Promise" : "an AsyncIterable"}. The value would be stored as-is and ` +
178+
`${thenable ? "a Promise" : "an AsyncIterable"}. The value would be stored as-is and ` +
170179
`never awaited in production; remove \`sync: true\` to use async-aware behavior, or ` +
171180
`unwrap the value before returning.`;
172181
emitDiagnostic({
@@ -243,8 +252,10 @@ export function handleAsync<T>(
243252
then?.();
244253
};
245254

246-
if (isThenable) {
255+
if (thenable) {
247256
let resolved = false,
257+
rejected = false,
258+
syncError: any,
248259
isSync = true;
249260
(result as PromiseLike<T>).then(
250261
v => {
@@ -254,11 +265,20 @@ export function handleAsync<T>(
254265
} else asyncWrite(v);
255266
},
256267
e => {
257-
if (!isSync) handleError(e);
268+
if (isSync) {
269+
syncError = e;
270+
rejected = true;
271+
} else handleError(e);
258272
}
259273
);
260274
isSync = false;
261-
if (!resolved) {
275+
if (rejected) {
276+
// Settle through the same status path an async rejection uses, then
277+
// unwind the in-progress synchronous read so the errored node isn't
278+
// momentarily read as `undefined`.
279+
handleError(syncError);
280+
throw syncError;
281+
} else if (!resolved) {
262282
globalQueue.initTransition(resolveTransition(el as any));
263283
throw new NotReadyError(context!);
264284
}
@@ -274,9 +294,7 @@ export function handleAsync<T>(
274294
completed = true;
275295
try {
276296
const returned = it.return?.();
277-
if (returned && typeof (returned as PromiseLike<IteratorResult<T>>).then === "function") {
278-
(returned as PromiseLike<IteratorResult<T>>).then(undefined, () => {});
279-
}
297+
if (isThenable(returned)) returned.then(undefined, () => {});
280298
} catch {}
281299
});
282300

packages/solid-signals/tests/action.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ describe("action", () => {
135135
expect(receivedValue).toBe(42);
136136
});
137137

138+
it("should await a yielded non-Promise thenable and resume with its value (#2765)", async () => {
139+
let receivedValue: any;
140+
141+
// Promise-like, but not `instanceof Promise` (custom thenable / cache
142+
// wrapper / cross-realm promise). `await` would wait for this; so must
143+
// the action, instead of resuming with the raw object.
144+
const thenable = {
145+
then(onFulfilled: (v: number) => void) {
146+
queueMicrotask(() => onFulfilled(42));
147+
}
148+
};
149+
150+
const myAction = action(function* () {
151+
receivedValue = yield thenable as any;
152+
});
153+
154+
myAction();
155+
expect(receivedValue).toBeUndefined();
156+
157+
await new Promise(resolve => queueMicrotask(() => resolve(undefined)));
158+
await Promise.resolve();
159+
expect(receivedValue).toBe(42);
160+
});
161+
138162
it("should handle multiple async yields in sequence", async () => {
139163
const values: number[] = [];
140164

packages/solid-signals/tests/syncThenable.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
createErrorBoundary,
23
createMemo,
34
createRenderEffect,
45
createRoot,
@@ -89,6 +90,35 @@ describe("sync thenable support", () => {
8990
expect(value()).toBe(undefined);
9091
});
9192

93+
it("should surface a synchronously-rejecting thenable to the error boundary (#2764)", () => {
94+
// A thenable (e.g. a cache that already knows it failed) that invokes its
95+
// rejection handler synchronously during `.then()` must settle the error,
96+
// not stay stuck on the pending path.
97+
function syncRejectingThenable(reason: unknown): PromiseLike<never> {
98+
return {
99+
then<R1, R2 = never>(
100+
_onfulfilled?: ((v: never) => R1 | PromiseLike<R1>) | null,
101+
onrejected?: ((reason: any) => R2 | PromiseLike<R2>) | null
102+
): PromiseLike<R1 | R2> {
103+
onrejected?.(reason);
104+
return syncThenable(undefined as any);
105+
}
106+
};
107+
}
108+
109+
const result = createRoot(() =>
110+
createErrorBoundary(
111+
() => {
112+
const value = createMemo(() => syncRejectingThenable(new Error("sync-reject")));
113+
return value();
114+
},
115+
err => `errored: ${(err() as Error).message}`
116+
)
117+
);
118+
119+
expect(result()).toBe("errored: sync-reject");
120+
});
121+
92122
it("should ignore stale thenable resolution during invalidation cleanup", () => {
93123
const pending = controlledThenable<number>();
94124
const [$source, setSource] = createSignal(0);

0 commit comments

Comments
 (0)