From b26b38d3477b4bf718d45521307b46179bcdfe1e Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Fri, 25 Sep 2026 09:42:57 -0700 Subject: [PATCH] fix(solid/server): derived async computations over a client hole classify FINAL and hand off instead of hanging (#3659) --- .../fix-client-hole-derived-async-hang.md | 6 + packages/solid/src/server/hydration.ts | 41 ++- packages/solid/src/server/signals.ts | 53 +++- packages/solid/test/server/ssr-async.spec.ts | 290 +++++++++++++++++- packages/web/src/index.server.ts | 8 +- .../client-hole-derived-async-3659.spec.tsx | 168 ++++++++++ 6 files changed, 551 insertions(+), 15 deletions(-) create mode 100644 .changeset/fix-client-hole-derived-async-hang.md create mode 100644 packages/web/test/server/client-hole-derived-async-3659.spec.tsx diff --git a/.changeset/fix-client-hole-derived-async-hang.md b/.changeset/fix-client-hole-derived-async-hang.md new file mode 100644 index 000000000..c0bf6a296 --- /dev/null +++ b/.changeset/fix-client-hole-derived-async-hang.md @@ -0,0 +1,6 @@ +--- +"solid-js": patch +"@solidjs/web": patch +--- + +Fix SSR hanging when a derived async computation reads a bare `ssrSource: "client"` hole inside ``. `createMemo(async …)`, `createProjection`, and `dynamic({ deferStream: true })` whose compute throws the client-hole NotReady now classify FINAL — the boundary hands the subtree off to the client exactly as a direct read does — instead of subscribing a retry to a source that never settles and leaving the response open. A client hole that surfaces before the shell has flushed now takes the same fallback + `$$f` route as one found at discovery, rather than rejecting the fragment over an empty region. diff --git a/packages/solid/src/server/hydration.ts b/packages/solid/src/server/hydration.ts index 1d6f6ed96..b83c1de79 100644 --- a/packages/solid/src/server/hydration.ts +++ b/packages/solid/src/server/hydration.ts @@ -497,13 +497,25 @@ function ssrLoadingBoundary( const finalAtDiscovery = ctx.async && hasFinalHole(); const fallbackOwner = createOwner({ id }); + // The placeholder wrapper around a streaming fallback (see `plainFallback`). + const tpl = collapseFallback + ? [``] + : [``, ``]; const fallbackResult = runWithOwner(fallbackOwner, () => { if (!ctx.async || finalAtDiscovery) return fallback(); - const tpl = collapseFallback - ? [``] - : [``, ``]; return ctx.ssr(tpl, ctx.escape(fallback())); }); + // The streaming fallback without its placeholder wrapper — what the + // "$$f" route inlines. The wrapper is ours (`tpl`), so the markup between + // its two halves is exactly the fallback. A resolved template is one + // segment (`t` a string, or a single-segment array — `h.length + 1`); + // more segments mean the fallback itself is still resolving (an async + // hole in it) and there is no plain markup to inline: `undefined`. + const plainFallback = (): string | undefined => { + const raw = (fallbackResult as any)?.t; + const t = Array.isArray(raw) ? (raw.length === 1 ? raw[0] : undefined) : raw; + return typeof t === "string" ? t.slice(tpl[0].length, t.length - tpl[1].length) : undefined; + }; if (finalAtDiscovery) { commitBoundaryState(); @@ -518,14 +530,27 @@ function ssrLoadingBoundary( if (ctx.async) { const regOpts = revealGroup ? { revealGroup: revealGroup.id } : undefined; done = ctx.registerFragment(id, regOpts); - // A final hole surfacing only now (an earlier real async read masked it - // during the initial discovery) can't take the "$$f" route anymore: the - // fragment protocol requires a settle, and "settle but keep the fallback" - // is not expressible. Reject instead — the placeholder swaps out and the - // client renders this boundary's content fresh after hydration + // A final hole surfacing only now: an earlier real async read masked it + // during the initial discovery, or the hole was reached through a + // derived async computation, whose FINAL classification lands a + // microtask after discovery (#3659). Before the shell has flushed the + // position is still the shell's to shape: the placeholder inlines away to + // the PLAIN fallback and the boundary serializes "$$f" — the at-discovery + // route, one pass late — with the fragment settling clean (the client's + // "$$f" branch takes precedence over a settled `_fr`). After the flush + // "settle but keep the fallback" is not expressible: the fragment + // protocol requires a swap. Reject instead — the placeholder swaps out + // and the client renders this boundary's content fresh after hydration // (resume(false)), the closest streaming analogue of the client-continue. const clientHandoff = () => { if (!flushed) commitBoundaryState(); + const plain = ctx.flushed !== undefined && !ctx.flushed() ? plainFallback() : undefined; + if (plain !== undefined) { + ctx.serialize(id, "$$f"); + done!(plain); + record("client", false); + return; + } const streamed = done!( undefined, new Error(`client-only content (bare ssrSource: "client")`) diff --git a/packages/solid/src/server/signals.ts b/packages/solid/src/server/signals.ts index 174d9f812..50fe9c106 100644 --- a/packages/solid/src/server/signals.ts +++ b/packages/solid/src/server/signals.ts @@ -1032,6 +1032,33 @@ function settleServerAsync( ) { let first = true; + // A pending read inside the compute. A real source's NotReady joins the + // retry chain: the attempt re-runs once it settles. A CLIENT HOLE's is + // FINAL for this computation (#3659): the compute derives from a value the + // server can never have, so the node has no server answer and never will. + // Retrying is pointless (the hole never settles), and leaving `deferred` + // pending wedged everything that waited on it — the enclosing + // (`Promise.all(pending.p)` over an untagged promise) and seroval's onDone + // for the serialized channel — so the response never completed. Classify + // the node as the bare client source it derives from: its error becomes + // the tagged client-hole NotReady (reads take that path — loud outside a + // boundary, FINAL inside one, where the boundary's re-pull sees the tag + // and hands off to the client), and the deferred resolves `undefined`, + // the abandonment ledger's value for a channel nobody consumes (the + // handed-off subtree renders fresh on the client, unhydrated). The tag is + // the classification (`hasFinalHole`'s rule), not identity: an + // aggregate over a hole carries it too. + const pending = (error: any): boolean => { + if (!(error instanceof NotReadyError)) return false; + if ((error.source as any)?.$clientHole === true) { + onError(new NotReadyError(CLIENT_HOLE)); + deferred.resolve(undefined as U); + return true; + } + subscribePendingRetry(error, attempt); + return true; + }; + const attempt = () => { if (isDisposed()) return; @@ -1040,7 +1067,7 @@ function settleServerAsync( current = first ? initial : rerun(); first = false; } catch (error) { - if (subscribePendingRetry(error, attempt)) return; + if (pending(error)) return; onError(error); deferred.reject(error); return; @@ -1057,9 +1084,10 @@ function settleServerAsync( }, error => { // NotReady defers to the retry chain (`attempt` no-ops once disposed — - // a re-created node joins the flight and drives the shared deferred). - // Terminal errors settle unconditionally, same as the success path. - if (subscribePendingRetry(error, attempt)) return; + // a re-created node joins the flight and drives the shared deferred) + // or, for a client hole, ends it (see `pending`). Terminal errors + // settle unconditionally, same as the success path. + if (pending(error)) return; onError(error); deferred.reject(error); } @@ -2302,7 +2330,13 @@ function createPendingProxy( let error: any; let readTarget: T = state; const gate = () => { - if (status > 1) throw error; + if (status > 1) { + // A derive that landed on a client hole (settleServerAsync's FINAL + // reclassification, #3659) errors with the tagged NotReady: the same + // loud-outside-a-boundary rule as the bare client store below. + if (isClientHole((error as NotReadyError)?.source)) clientHoleRead(); + throw error; + } if (status) return; // Bare client store: same loud-outside-a-boundary rule as the memo // read path (see clientHoleRead). @@ -2527,6 +2561,15 @@ export function createProjection( result = runProjection(); } catch (error) { if (!(error instanceof NotReadyError)) throw error; + // The derive read a client hole synchronously: FINAL at discovery, the + // structural form of a bare client projection (#3659) — no deferred to + // retry, no channel to serialize; the nearest boundary hands + // the position to the client at once. (A hole reached asynchronously — + // after an await, or on a retry — lands in settleServerAsync's FINAL + // reclassification instead.) + if ((error.source as any)?.$clientHole === true) { + return createPendingProxy(state, CLIENT_HOLE)[0]; + } const deferred = createDeferredPromise(); const [pending, markReady, markError] = createPendingProxy(state, deferred.promise); diff --git a/packages/solid/test/server/ssr-async.spec.ts b/packages/solid/test/server/ssr-async.spec.ts index 981034024..b3450356e 100644 --- a/packages/solid/test/server/ssr-async.spec.ts +++ b/packages/solid/test/server/ssr-async.spec.ts @@ -151,7 +151,9 @@ function deferred() { return { promise, resolve, reject }; } -function createMockSSRContext(options: { async?: boolean; fragmentFlushed?: boolean } = {}) { +function createMockSSRContext( + options: { async?: boolean; fragmentFlushed?: boolean; flushed?: boolean } = {} +) { const serialized = new Map(); const registeredFragments = new Set(); const fragmentResults = new Map(); @@ -185,6 +187,10 @@ function createMockSSRContext(options: { async?: boolean; fragmentFlushed?: bool }; } }; + // The renderer's "has the shell left?" probe (`ctx.flushed`). Absent by + // default, as the older tests were written; a boundary that consults it + // (the pre-flush client handoff) sees this answer when set. + if (options.flushed !== undefined) context.flushed = () => options.flushed; return { context, @@ -2021,6 +2027,47 @@ describe("ssrSource server modes", () => { expect([...serialized.values()]).not.toContain("$$f"); }); + test("streaming: final hole masked by a real await that settles PRE-FLUSH inlines the fallback + $$f", async () => { + const { context, serialized, registeredFragments, fragmentResults, fragmentErrors } = + createMockSSRContext({ flushed: false }); + sharedConfig.context = context; + + const d = deferred(); + let result: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const data = createMemo(() => d.promise); + const v = data(); + const widget = (createMemo as any)(() => 42, { ssrSource: "client" }); + return ssr( + ["
", "-", "
"], + () => v, + () => widget() + ) as any; + } + }); + }, + { id: "t" } + ); + expect(registeredFragments.size).toBe(1); + expect(result().t[0]).toContain("Shell"); + + d.resolve("real"); + await tick(); + + // The shell had not left when the hole surfaced: the position is still + // the shell's to shape, so this is the at-discovery route one pass + // late — plain fallback inlined, "$$f" serialized, fragment settled + // clean — not a rejected fragment over an empty region (#3659). + const bid = [...registeredFragments][0]; + expect(fragmentResults.get(bid)).toBe("Shell"); + expect(fragmentErrors.size).toBe(0); + expect(serialized.get(bid)).toBe("$$f"); + }); + test("renderToString: client hole takes the existing fallback + $$f route", () => { const { context, serialized, registeredFragments } = createMockSSRContext({ async: false }); sharedConfig.context = context; @@ -2185,6 +2232,247 @@ describe("ssrSource server modes", () => { }); }); + // #3659: a DERIVED ASYNC computation whose compute reads a client hole has + // no server answer and never will — it classifies FINAL like the bare + // source it derives from, and the boundary hands off to the client instead + // of awaiting a pending source that can never settle. Before the fix the + // derived node's own deferred (untagged, never settling) was what the + // boundary and the serialized channel waited on: the response never ended. + describe("derived async computations over a client hole (#3659)", () => { + const clientSource = () => + (createMemo as any)(() => [1, 2, 3], { ssrSource: "client" }) as () => number[]; + + test("async memo, pre-flush: the deferred settles, the boundary inlines the fallback + $$f", async () => { + const { context, serialized, registeredFragments, fragmentResults, fragmentErrors } = + createMockSSRContext({ flushed: false }); + sharedConfig.context = context; + + let result: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const client = clientSource(); + // The compute is async: its rejection — the tagged NotReady the + // hole read threw inside it — lands a microtask after discovery, + // so the fragment is already registered when the node turns FINAL. + const derived = createMemo(async () => client().length); + return ssr(["
", "
"], () => derived()) as any; + } + }); + }, + { id: "t" } + ); + + // Discovery saw an untagged pending source: streaming route, fragment registered. + expect(registeredFragments.size).toBe(1); + expect(result().t[0]).toContain("Shell"); + + await tick(); + + // The derived node's serialized channel settled (`undefined`, the + // abandonment ledger's value for a channel nobody consumes) — seroval + // can finish; and the boundary took the client-continue route with the + // shell still open: the placeholder inlined to the PLAIN fallback, "$$f" + // serialized, the fragment settled clean. + const bid = [...registeredFragments][0]; + const channel = [...serialized.entries()].find(([, v]) => v && typeof v.then === "function"); + expect(channel).toBeDefined(); + await expect(channel![1]).resolves.toBeUndefined(); + expect(serialized.get(bid)).toBe("$$f"); + expect(fragmentResults.get(bid)).toBe("Shell"); + expect(fragmentErrors.size).toBe(0); + }); + + test("async memo, post-flush: the fragment rejects as client-only content", async () => { + const { context, serialized, registeredFragments, fragmentResults, fragmentErrors } = + createMockSSRContext({ flushed: true }); + sharedConfig.context = context; + + let result: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const client = clientSource(); + const derived = createMemo(async () => client().length); + return ssr(["
", "
"], () => derived()) as any; + } + }); + }, + { id: "t" } + ); + expect(registeredFragments.size).toBe(1); + expect(result().t[0]).toContain("Shell"); + + await tick(); + + // Past the flush "settle but keep the fallback" is inexpressible: the + // existing late-handoff route — reject, the client renders the content + // fresh (resume(false)). No "$$f" on this route. + expect(fragmentResults.size).toBe(1); + expect([...fragmentResults.values()][0]).toBeUndefined(); + expect(String([...fragmentErrors.values()][0])).toMatch(/client-only content/); + expect([...serialized.values()]).not.toContain("$$f"); + }); + + test("async memo: the node's error becomes the tagged client-hole NotReady (FINAL on re-pull)", async () => { + const { context } = createSerializeTrackingContext(); + sharedConfig.context = context; + + let derived: any; + createRoot( + () => { + const client = clientSource(); + (context as any)._loadingPhase = true; + try { + derived = createMemo(async () => client().length); + } finally { + (context as any)._loadingPhase = undefined; + } + }, + { id: "t" } + ); + await tick(); + + // Inside a Loading pass: the tagged FINAL suspension, the same one a + // direct read of the source throws. + (context as any)._loadingPhase = true; + let caught: any; + try { + derived(); + } catch (e) { + caught = e; + } finally { + (context as any)._loadingPhase = undefined; + } + expect(caught).toBeInstanceOf(NotReadyError); + expect(caught.source.$clientHole).toBe(true); + // Outside one: the loud error, never a hang. + expect(() => derived()).toThrow(/outside a boundary/); + }); + + test("projection deriving synchronously from the hole is FINAL at discovery: $$f, no fragment", () => { + const { context, serialized, registeredFragments } = createMockSSRContext(); + sharedConfig.context = context; + + let result: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const client = clientSource(); + const proj = (createProjection as any)( + (d: any) => { + d.n = client().length; + }, + { n: 0 } + ); + return ssr(["
", "
"], () => proj.n) as any; + } + }); + }, + { id: "t" } + ); + + // The derive threw the hole synchronously: no deferred, no channel, the + // structural bare-client-projection form — handed off at once. + expect(registeredFragments.size).toBe(0); + expect([...serialized.values()]).toEqual(["$$f"]); + expect(result()).toBe("Shell"); + }); + + test("async projection (async derive) reclassifies FINAL and hands off; loud outside a boundary", async () => { + const { context, serialized, registeredFragments, fragmentResults, fragmentErrors } = + createMockSSRContext({ flushed: false }); + sharedConfig.context = context; + + let result: any; + let proj: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const client = clientSource(); + proj = (createProjection as any)( + async (d: any) => { + d.n = client().length; + }, + { n: 0 } + ); + return ssr(["
", "
"], () => proj.n) as any; + } + }); + }, + { id: "t" } + ); + expect(result().t[0]).toContain("Shell"); + + await tick(); + + const bid = [...registeredFragments][0]; + expect(serialized.get(bid)).toBe("$$f"); + expect(fragmentResults.get(bid)).toBe("Shell"); + expect(fragmentErrors.size).toBe(0); + // The pending proxy errored with the tagged NotReady; read outside a + // Loading pass it takes the bare client store's loud path. + expect(() => proj.n).toThrow(/outside a boundary/); + }); + + test("outside : an async memo over the hole errors loudly at its read, never hangs", async () => { + const { context } = createSerializeTrackingContext(); + sharedConfig.context = context; + + let derived: any; + createRoot( + () => { + const client = clientSource(); + // No loading pass: the hole read inside the async compute throws + // the loud error synchronously (before any await) — the compute's + // promise rejects with it, and the memo surfaces it as a real error. + derived = createMemo(async () => client().length); + }, + { id: "t" } + ); + await tick(); + expect(() => derived()).toThrow(/ASYNC_OUTSIDE_LOADING_BOUNDARY/); + }); + + test("a real (server-fillable) async dependency still retries and lands", async () => { + const { context, registeredFragments, fragmentResults } = createMockSSRContext({ + flushed: false + }); + sharedConfig.context = context; + + const d = deferred(); + let result: any; + createRoot( + () => { + result = Loading({ + fallback: "Shell", + get children() { + const data = createMemo(() => d.promise); + const derived = createMemo(async () => data().length); + return ssr(["
", "
"], () => derived()) as any; + } + }); + }, + { id: "t" } + ); + expect(registeredFragments.size).toBe(1); + + d.resolve([1, 2]); + await tick(); + await tick(); + + expect(fragmentResults.get([...registeredFragments][0])).toBe("
2
"); + }); + }); + test("ssrSource 'hybrid' runs computation (same as default for Promises)", () => { const { context, serializeLog } = createSerializeTrackingContext(); sharedConfig.context = context; diff --git a/packages/web/src/index.server.ts b/packages/web/src/index.server.ts index c8bb850eb..cd5e43c18 100644 --- a/packages/web/src/index.server.ts +++ b/packages/web/src/index.server.ts @@ -160,7 +160,13 @@ export function dynamic( // Hold the shell on the source once per instance. A no-op after the // shell has flushed, like every blocker; a rejection is the memo's // to surface on the retry, the block only needs to clear. - if (!gated && err instanceof NotReadyError) { + // + // Never on a client hole (a bare `ssrSource: "client"` read in the + // source, #3659): FINAL — the server can never fill it, so a block + // on it would hold the shell forever. Rethrow untouched: the + // enclosing discovery pass reads the tag and hands the + // position to the client (the same rule `serverEffect` applies). + if (!gated && err instanceof NotReadyError && !(err.source as any)?.$clientHole) { gated = true; ctx.block( Promise.resolve(err.source).then( diff --git a/packages/web/test/server/client-hole-derived-async-3659.spec.tsx b/packages/web/test/server/client-hole-derived-async-3659.spec.tsx new file mode 100644 index 000000000..6be35db8e --- /dev/null +++ b/packages/web/test/server/client-hole-derived-async-3659.spec.tsx @@ -0,0 +1,168 @@ +/** + * @jsxImportSource @solidjs/web + */ +// #3659: a derived ASYNC computation whose compute reads a bare +// `ssrSource: "client"` source inside `` must classify FINAL and +// hand the position to the client, the same as a direct or sync-derived +// read does. Before the fix the derived computation's own pending source +// (its deferred / the shell blocker) was a fresh untagged promise that could +// never settle: the boundary awaited it forever, seroval awaited the +// serialized channel forever, and the response never completed. +// +// Pinned against the real renderer: each shape's stream must END. A hang +// here is the test's timeout — the assertion is completion, not a duration. +import { describe, expect, test } from "vitest"; +import { Loading, dynamic, renderToStream } from "@solidjs/web"; +import { createMemo, createProjection, OBSERVE, type BoundaryEvent } from "solid-js"; + +/** Streams through a sink like a response does; resolves with everything written. */ +function stream(code: () => any): Promise { + return new Promise(resolve => { + const chunks: string[] = []; + renderToStream(code).pipe({ + write(chunk: string) { + chunks.push(chunk); + }, + end() { + resolve(chunks.join("")); + } + }); + }); +} + +function boundaryRecords() { + const seen: BoundaryEvent[] = []; + const off = OBSERVE!.records.subscribe("boundary", event => { + seen.push(event); + }); + return { seen, off }; +} + +const clientSource = () => + (createMemo as any)(() => Promise.resolve([1, 2, 3]), { ssrSource: "client" }) as () => number[]; + +describe("derived async computations over a client hole hand off instead of hanging (#3659)", () => { + test("control: a SYNC derived memo over the hole hands off at discovery", async () => { + const { seen, off } = boundaryRecords(); + function Derived() { + const client = clientSource(); + const derived = createMemo(() => client().length); + return
{derived()}
; + } + const html = await stream(() => ( + loading}> + + + )); + off(); + expect(html).toContain("loading"); + expect(seen.map(e => e.outcome)).toEqual(["client"]); + }); + + test("async memo: createMemo(async () => client().length) completes and hands off", async () => { + const { seen, off } = boundaryRecords(); + function Derived() { + const client = clientSource(); + const derived = createMemo(async () => client().length); + return
{derived()}
; + } + const html = await stream(() => ( + loading}> + + + )); + off(); + expect(html).toContain("loading"); + // The client renders the content: no server-rendered `
3
`. + expect(html).not.toContain("
3
"); + expect(seen.map(e => e.outcome)).toEqual(["client"]); + }); + + test("projection: createProjection(d => { d.n = client().length }) completes and hands off", async () => { + const { seen, off } = boundaryRecords(); + function Derived() { + const client = clientSource(); + const proj = createProjection( + (d: { n: number }) => { + d.n = client().length; + }, + { n: 0 } + ); + return
{proj.n}
; + } + const html = await stream(() => ( + loading}> + + + )); + off(); + expect(html).toContain("loading"); + expect(html).not.toContain("
3
"); + expect(seen.map(e => e.outcome)).toEqual(["client"]); + }); + + test("dynamic({ deferStream }): a client-hole source never blocks the shell; the boundary hands off", async () => { + const { seen, off } = boundaryRecords(); + function Derived() { + const client = clientSource(); + const Dyn = dynamic(() => (client().length ? "span" : "b"), { deferStream: true }); + return x; + } + const html = await stream(() => ( + loading}> + + + )); + off(); + expect(html).toContain("loading"); + expect(html).not.toContain(" e.outcome)).toEqual(["client"]); + }); + + test("control: dynamic({ deferStream }) over a REAL async source still holds the shell for it", async () => { + function Derived() { + const tag = createMemo(async () => { + await new Promise(r => setTimeout(r, 5)); + return "span"; + }); + const Dyn = dynamic(() => tag() as any, { deferStream: true }); + return x; + } + const html = await stream(() => ( + loading}> + + + )); + // deferStream: the shell waited for the source, so the content is inline + // in the shell — not behind a streamed fragment. + expect(html).toContain(": a derived async read of the hole is still the loud error, not a hang", async () => { + const errors: unknown[] = []; + function Derived() { + const client = clientSource(); + const derived = createMemo(async () => client().length); + return
{derived()}
; + } + const html = await new Promise(resolve => { + const chunks: string[] = []; + renderToStream(() => , { + onError(err: unknown) { + errors.push(err); + } + } as any).pipe({ + write(chunk: string) { + chunks.push(chunk); + }, + end() { + resolve(chunks.join("")); + } + }); + }); + expect(errors.length).toBeGreaterThan(0); + expect(String(errors[0])).toMatch(/ASYNC_OUTSIDE_LOADING_BOUNDARY/); + expect(html).not.toContain("
3
"); + }); +});