From f98bd77bbec4221bf88f44ff18d6030acc47eb03 Mon Sep 17 00:00:00 2001 From: Brenley Dueck Date: Mon, 7 Sep 2026 16:55:40 -0500 Subject: [PATCH 1/2] fix(signals): hold conditional reveals on existing async work --- .../fix-conditional-async-reveal-pending.md | 5 + packages/signals/src/core/scheduler.ts | 11 ++ .../tests/spec-async-semantics.test.ts | 139 ++++++++++++++++++ packages/web/test/loading.spec.tsx | 49 ++++++ 4 files changed, 204 insertions(+) create mode 100644 .changeset/fix-conditional-async-reveal-pending.md diff --git a/.changeset/fix-conditional-async-reveal-pending.md b/.changeset/fix-conditional-async-reveal-pending.md new file mode 100644 index 000000000..888f73187 --- /dev/null +++ b/.changeset/fix-conditional-async-reveal-pending.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Hold conditional reveals that first observe async work started in an earlier flush. The revealing signal now stays pending until the details can commit with it, including when the reveal creates a new child reader. Preserve fresh/reset loading-boundary fallbacks and avoid opening a second transition for readers already waiting in one. diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index 84033c666..bb417d91d 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -755,6 +755,17 @@ export class GlobalQueue extends Queue { // to completion accounting BY CONSTRUCTION: it never registers a // reporter and never counts toward the loading-boundary diagnostic. if ((actualError as NotReadyError)?._markVisual) return true; + // A reveal can discover a flight started in an earlier flush. Hold the + // staged writes with that reader (A15), even if the reader is new. + // Fresh/reset loading boundaries consume pending before it reaches here. + // A reader already parked in a transition must not open a second one. + if ( + !activeTransition && + !node._transition && + actualError && + this._batch._pendingNodes.length + ) + this.initTransition(); if (activeTransition && actualError) { const source = (actualError as NotReadyError).source; // The one sanctioned registration site (INV-3): async blockers only diff --git a/packages/signals/tests/spec-async-semantics.test.ts b/packages/signals/tests/spec-async-semantics.test.ts index 394a421a2..149bfb4e0 100644 --- a/packages/signals/tests/spec-async-semantics.test.ts +++ b/packages/signals/tests/spec-async-semantics.test.ts @@ -198,6 +198,145 @@ describe("A15 (was B3): overlapping transitions settle as one unit", () => { // a *shared* reader join one transition and settle together; writes on // fully disjoint graphs keep independent transitions and settle // independently. Both halves are the spec. + it.each([ + { child: false, boundary: false }, + { child: false, boundary: true }, + { child: true, boundary: false }, + { child: true, boundary: true } + ])( + "holds a reveal of an existing async flight (child=$child, boundary=$boundary)", + async ({ child, boundary }) => { + const [value, setValue] = createSignal(0); + const [show, setShow] = createSignal(false); + const fetcher = deferredFetcher(v => v); + let valueLine: readonly [number, boolean] | undefined; + let showLine: readonly [boolean, boolean] | undefined; + let slot: number | string | undefined; + const childLog: number[] = []; + let dispose!: () => void; + + createRoot(d => { + dispose = d; + const details = createMemo(() => fetcher.fetch(value())); + createRenderEffect( + () => [value(), isPending(value)] as const, + v => { + valueLine = v; + } + ); + createRenderEffect( + () => [show(), isPending(show)] as const, + v => { + showLine = v; + } + ); + const content = () => { + if (!show()) return "hidden"; + if (!child) return details(); + createRenderEffect(details, v => { + childLog.push(v); + }); + return "shown"; + }; + const read = boundary ? createLoadingBoundary(content, () => "loading") : content; + createRenderEffect(read, v => { + slot = v; + }); + }); + + try { + flush(); + expect([valueLine, showLine, slot]).toEqual([[0, false], [false, false], "hidden"]); + + // Cover both a never-resolved source and a refetch after a previous reveal. + for (const next of [1, 2]) { + setShow(false); + flush(); + setValue(next); + flush(); + expect([valueLine, showLine, slot]).toEqual([[next, false], [false, false], "hidden"]); + const previousChildCount = childLog.length; + + // The flight started in the earlier flush, not during this reveal. + setShow(true); + flush(); + expect([valueLine, showLine, slot]).toEqual([[next, false], [false, true], "hidden"]); + expect(show()).toBe(false); + expect(isPending(value)).toBe(false); + expect(isPending(show)).toBe(true); + expect(childLog).toHaveLength(previousChildCount); + + fetcher.resolveAll(); + await settle(); + expect([valueLine, showLine, slot]).toEqual([ + [next, false], + [true, false], + child ? "shown" : next + ]); + expect(show()).toBe(true); + expect(isPending(show)).toBe(false); + if (child) expect(childLog[childLog.length - 1]).toBe(next); + } + } finally { + dispose(); + } + } + ); + + it.each(["new", "reset"])( + "lets a %s loading boundary catch an existing flight without holding the reveal", + async mode => { + const [value, setValue] = createSignal(0); + const [show, setShow] = createSignal(false); + const fetcher = deferredFetcher(v => v); + let showLine: readonly [boolean, boolean] | undefined; + let slot: number | string | undefined; + let dispose!: () => void; + + createRoot(d => { + dispose = d; + const details = createMemo(() => fetcher.fetch(value())); + createRenderEffect( + () => [show(), isPending(show)] as const, + v => { + showLine = v; + } + ); + const read = + mode === "reset" + ? createLoadingBoundary( + () => (show() ? details() : "hidden"), + () => "loading", + { on: show } + ) + : () => (show() ? createLoadingBoundary(details, () => "loading")() : "hidden"); + createRenderEffect(read, v => { + slot = v; + }); + }); + + try { + flush(); + expect([showLine, slot]).toEqual([[false, false], "hidden"]); + setValue(1); + flush(); + expect(value()).toBe(1); + + setShow(true); + flush(); + expect([showLine, slot]).toEqual([[true, false], "loading"]); + expect(show()).toBe(true); + expect(isPending(show)).toBe(false); + + fetcher.resolveAll(); + await settle(); + expect([showLine, slot]).toEqual([[true, false], 1]); + } finally { + dispose(); + } + } + ); + it("a shared reader forces one settle point: nothing commits until both asyncs resolve", async () => { const [a, setA] = createSignal(1); const [b, setB] = createSignal(1); diff --git a/packages/web/test/loading.spec.tsx b/packages/web/test/loading.spec.tsx index 0f1eb09fc..e2d4651cb 100644 --- a/packages/web/test/loading.spec.tsx +++ b/packages/web/test/loading.spec.tsx @@ -658,6 +658,55 @@ describe("Testing Loading", () => { localDispose(); }); + test("a conditional reveal waits for an async flight from an earlier flush", async () => { + const localDiv = document.createElement("div"); + let click!: () => void; + const localDispose = render(() => { + const [value, setValue] = createSignal(0); + const [show, setShow] = createSignal(false); + const details = createMemo(async () => { + const v = value(); + await new Promise(r => setTimeout(r, 3000)); + return v; + }); + click = () => { + setValue(v => v + 1); + setTimeout(() => setShow(true), 1000); + }; + return ( +
+
+ value(): {value()} {isPending(value) ? "pending" : null} +
+
+ show(): {String(show())} {isPending(show) ? "pending" : null} +
+
{show() ? details() : "hidden"}
+
+ ); + }, localDiv); + const lines = () => + Array.from(localDiv.firstElementChild!.children, el => el.textContent!.trim()); + + try { + flush(); + expect(lines()).toEqual(["value(): 0", "show(): false", "hidden"]); + click(); + flush(); + expect(lines()).toEqual(["value(): 1", "show(): false", "hidden"]); + + await vi.advanceTimersByTimeAsync(1000); + flush(); + expect(lines()).toEqual(["value(): 1", "show(): false pending", "hidden"]); + + await vi.advanceTimersByTimeAsync(2000); + flush(); + expect(lines()).toEqual(["value(): 1", "show(): true", "1"]); + } finally { + localDispose(); + } + }); + test("implicit route transition stays held after lazy component is cached", async () => { let setRoute!: (value: "home" | "profile") => void; const localDiv = document.createElement("div"); From 98d457f82c1705d6098aeb0b1cabb3c50d7cc7f9 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Mon, 7 Sep 2026 22:13:53 -0700 Subject: [PATCH 2/2] refactor(signals): fold the reveal-hold check into the existing actualError branch Same semantics, fewer bytes: the new check and the reporter registration share one actualError test, read the module-level currentBatch instead of this._batch, and the error fallback is `??` (every caller passes nothing or this node's own _x._error, so a null falls back to the same null). Measured against next: signals + createStore 14358 B brotli, under both the cap and the pre-PR baseline; the PR as written was 18 B over on Linux. Co-authored-by: Cursor --- packages/signals/src/core/scheduler.ts | 50 +++++++++++++------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index bb417d91d..5380848d8 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -749,36 +749,36 @@ export class GlobalQueue extends Queue { // Only track async if the boundary is propagating STATUS_PENDING (not caught by boundary) if (mask & STATUS_PENDING) { if (flags & STATUS_PENDING) { - const actualError = error !== undefined ? error : node._x?._error; + // Callers pass either nothing or this node's own `_x._error`, so `??` + // is exact (a null error falls back to the same null). + const actualError = error ?? node._x?._error; // A visibility-only mark notification (the affects() boundary // channel) updates display state on its way up but must be invisible // to completion accounting BY CONSTRUCTION: it never registers a // reporter and never counts toward the loading-boundary diagnostic. if ((actualError as NotReadyError)?._markVisual) return true; - // A reveal can discover a flight started in an earlier flush. Hold the - // staged writes with that reader (A15), even if the reader is new. - // Fresh/reset loading boundaries consume pending before it reaches here. - // A reader already parked in a transition must not open a second one. - if ( - !activeTransition && - !node._transition && - actualError && - this._batch._pendingNodes.length - ) - this.initTransition(); - if (activeTransition && actualError) { - const source = (actualError as NotReadyError).source; - // The one sanctioned registration site (INV-3): async blockers only - // enter the transition from queue notification. - if (__DEV__) beginAsyncReporterWrites(); - let reporters = activeTransition._asyncReporters.get(source); - if (!reporters) activeTransition._asyncReporters.set(source, (reporters = new Set())); - if (__DEV__) endAsyncReporterWrites(); - const prevSize = reporters.size; - reporters.add(node); - if (reporters.size !== prevSize) { - schedule(); - GlobalQueue._wakeSuppressedProbes?.(activeTransition); + if (actualError) { + // A reveal can discover a flight started in an earlier flush. Hold + // the staged writes with that reader (A15), even if the reader is + // new. Fresh/reset loading boundaries consume pending before it + // reaches here. A reader already parked in a transition must not + // open a second one. + if (!activeTransition && !node._transition && currentBatch._pendingNodes.length) + this.initTransition(); + if (activeTransition) { + const source = (actualError as NotReadyError).source; + // The one sanctioned registration site (INV-3): async blockers only + // enter the transition from queue notification. + if (__DEV__) beginAsyncReporterWrites(); + let reporters = activeTransition._asyncReporters.get(source); + if (!reporters) activeTransition._asyncReporters.set(source, (reporters = new Set())); + if (__DEV__) endAsyncReporterWrites(); + const prevSize = reporters.size; + reporters.add(node); + if (reporters.size !== prevSize) { + schedule(); + GlobalQueue._wakeSuppressedProbes?.(activeTransition); + } } } if (__DEV__ && _enforceLoadingBoundary) _hitUnhandledAsync = true;