From af55363749f9a861d26c0f78e407f26be5f8dec9 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 6 Oct 2026 02:06:29 -0700 Subject: [PATCH] fix(web/frames): classify an adopted occurrence only after every delivered record has drained (C18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frames-rulings 3.5 (proposed): "an occurrence is classified only after every delivered record has drained" — pending is the drain's state, not the parser's. `adoptBoundary.recordsPending` gains a third term: `_$HY.r` holds a slot/region record for the boundary not yet in `appliedRecords`. Before: two records drained after the parser finished classified each other — `drainRecords` applies one record per `host.apply`, each a synchronous `#flush` → `#syncSlots`; the first apply's sync found the second occurrence recordless with `readyState` no longer "loading", classified it direct-insert and evaluated its render prop argless: `TypeError` → REACTIVITY_HALTED. Same window via a live-hole op and the live pump's catch-up read (contract C18 / R9). Pins: harness/replay C18 ×3 flipped from test.fails to test; c18-classify-after-drain.spec.tsx pins the consequence with a real fill (record drain; live op in the window; control). Co-authored-by: Claude via Cursor --- .changeset/frames-c18-classify-after-drain.md | 5 + packages/web/frames/src/client.ts | 74 +++-- packages/web/frames/src/frame-client.ts | 11 +- .../c18-classify-after-drain.spec.tsx | 253 ++++++++++++++++++ .../test/consistency/harness/replay.spec.tsx | 95 ++++--- 5 files changed, 363 insertions(+), 75 deletions(-) create mode 100644 .changeset/frames-c18-classify-after-drain.md create mode 100644 packages/web/test/consistency/c18-classify-after-drain.spec.tsx diff --git a/.changeset/frames-c18-classify-after-drain.md b/.changeset/frames-c18-classify-after-drain.md new file mode 100644 index 000000000..e89eee687 --- /dev/null +++ b/.changeset/frames-c18-classify-after-drain.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Frames client: a recordless adopted occurrence is classified only after every delivered document record has drained (frames-rulings 3.5, proposed; consistency contract C18 / red R9). `adoptBoundary.recordsPending` gains a third term — `_$HY.r` still holds a slot or region record for the boundary that `drainRecords` has not applied — so "pending" is the drain's state, not only the parser's. Before, two records drained after the parser finished classified each other: the deferred drain applies one record per `host.apply`, each a synchronous frame sync, and that first sync found the second occurrence recordless with `document.readyState` no longer "loading", classified it direct-insert, and evaluated its render prop as a zero-arg accessor — a `TypeError` on the props read halted the reactive system. The same window was reachable from a live-hole op and from the live pump's catch-up read. Now such a sync defers the occurrence and the drain's next apply mounts it with its args. diff --git a/packages/web/frames/src/client.ts b/packages/web/frames/src/client.ts index 6634f8994..0a6bcfc14 100644 --- a/packages/web/frames/src/client.ts +++ b/packages/web/frames/src/client.ts @@ -1331,6 +1331,11 @@ function adoptBoundary( // recordless occurrence it deferred (#2968 — the frame's recordsPending/ // drainRecords seam below). const appliedRecords = new Set(); + // The document keys this boundary's records under the wire name: slot + // records as `sc:slot::`, nested regions as + // `sc:region:..`. + const slotPrefix = `sc:slot:${id}:`; + const regionPrefix = `sc:region:${id}.`; // Deferred fragments in the adopted markup (#2978): a that // suspended inside the server component during document SSR left a `pl-*` // placeholder here, but its producer ran on the SERVER — no client @@ -1361,7 +1366,9 @@ function adoptBoundary( // drain normally starts the pump; attempted on every re-drain anyway — // idempotent, and a defensive catch for a record that lands late. pumpLiveChannel(); - const slotPrefix = `sc:slot:${id}:`; + // Each record is marked applied BEFORE its `host.apply`: that apply + // syncs the frame, and the sync's `recordsPending` must read the other + // delivered records as still pending while this one is no longer (3.5). for (const key of Object.keys(hy.r)) { if (appliedRecords.has(key)) continue; if (key.startsWith(slotPrefix)) { @@ -1375,19 +1382,17 @@ function adoptBoundary( key: key.slice(slotPrefix.length), args: hy.r[key] }); - } else if (key.startsWith("sc:region:")) { + } else if (key.startsWith(regionPrefix)) { + appliedRecords.add(key); + // Async-occluded regions arrive as promises (the producer held + // the stream on them); regions keep their producer-relative ids + // (the records reference them by those), and the store warms per + // id either way, so a late apply still lands before the region + // binds on expand. const childId = key.slice("sc:region:".length); - if (childId.startsWith(id + ".")) { - appliedRecords.add(key); - // Async-occluded regions arrive as promises (the producer held - // the stream on them); regions keep their producer-relative ids - // (the records reference them by those), and the store warms per - // id either way, so a late apply still lands before the region - // binds on expand. - const val = hy.r[key]; - const apply = (html: any) => host.apply({ type: "html", id: childId, version: 0, html }); - val && typeof val.then === "function" ? val.then(apply) : apply(val); - } + const val = hy.r[key]; + const apply = (html: any) => host.apply({ type: "html", id: childId, version: 0, html }); + val && typeof val.then === "function" ? val.then(apply) : apply(val); } } }; @@ -1473,23 +1478,44 @@ function adoptBoundary( ownerScope: boundaryScope(owner), reveal: revealSeam(owner), onApply: settle, - // May the document still run scripts that assign records? While the - // parser is running the answer is yes, and a held fragment's replay can - // still deliver one — so a recordless occurrence defers instead of - // misclassifying as content (the runtime re-checks until this flips - // false). Deliberately NOT boundaryMayArrive(): its `!_$HY.done` term - // answers a different question (can this boundary's ELEMENT still - // appear), and holding classification until client hydration completes - // pushes the adopted mount past the hydrate window — the claim then - // adopts markup the client's state has already moved past (the - // adopted-slot-live spec pins the working ordering). + // Is a record for this boundary still to come — or here and not yet + // applied? While the parser is running the document can still run a + // data script, a held fragment's replay can still deliver one, and a + // record whose script already ran sits in `_$HY.r` until the drain + // moves it into the store — so a recordless occurrence defers instead + // of misclassifying as content (the runtime re-checks until this flips + // false). The third term is frames-rulings 3.5 (contract C18/R9): an + // occurrence is classified only after every delivered record has + // drained — "pending" is the drain's state, not the parser's. Without + // it, two records drained after the parser finished classify each + // other: the first apply's sync finds the second recordless with the + // parser done, evaluates its render prop argless, and the `TypeError` + // halts the reactive system. Deliberately NOT boundaryMayArrive(): its + // `!_$HY.done` term answers a different question (can this boundary's + // ELEMENT still appear), and holding classification until client + // hydration completes pushes the adopted mount past the hydrate window + // — the claim then adopts markup the client's state has already moved + // past (the adopted-slot-live spec pins the working ordering). // Spread-cast: the published FrameOptions predates this seam; a runtime // without it simply never calls the hooks (drop once the pin catches up). ...({ recordsPending: () => { if (document.readyState === "loading") return true; const hy = (globalThis as any)._$HY; - return !!(hy && hy.fr && hy.fr.pending()); + if (!hy) return false; + if (hy.fr && hy.fr.pending()) return true; + // Delivered and undrained: a record for this boundary whose data + // script ran (`_$HY.r` has the key) that the drain has not moved + // into the store yet. "Recordless" reads the store, and the parser's + // state says nothing about this gap — the record sits one loop + // iteration from applying while a sync runs. + for (const key in hy.r) + if ( + !appliedRecords.has(key) && + (key.startsWith(slotPrefix) || key.startsWith(regionPrefix)) + ) + return true; + return false; }, drainRecords, // The identity split binds the frame to the call ADDRESS (id + args diff --git a/packages/web/frames/src/frame-client.ts b/packages/web/frames/src/frame-client.ts index d28248b1f..5fd5a801d 100644 --- a/packages/web/frames/src/frame-client.ts +++ b/packages/web/frames/src/frame-client.ts @@ -347,7 +347,10 @@ export interface FrameOptions { * occurrence is ambiguous while this returns true: the frame defers its * mount one macrotask (all currently parsed scripts run first), calls * `drainRecords`, and classifies with whatever is then resolvable. Return - * false once the document can run no further data scripts. + * false once the document can run no further data scripts AND every + * record it has already delivered has been drained (frames-rulings 3.5): + * a record that executed but has not been applied yet is pending too — a + * sync in that window must defer, not classify. */ recordsPending?(): boolean; /** Re-absorb the document's arrived-by-now records (idempotent per key). */ @@ -1312,7 +1315,11 @@ class FrameImpl { // bounded by the same contract as everything else here: // recordsPending flips false when the document completes with no // fragment left to reveal (truncation included — the ledger rejects - // stragglers). Deferral is invisible on screen: an adopted + // stragglers) and no delivered record left undrained (frames- + // rulings 3.5: the drain itself syncs once per record it applies, + // and that sync must not classify the records still in its loop — + // nor may a live op's sync in the same window). Deferral is + // invisible on screen: an adopted // occurrence's server-rendered interior is already in the DOM; the // mount is the hydration attach. Full syncs only: a scoped segment // fill renders into a detached fragment a later full sync can't diff --git a/packages/web/test/consistency/c18-classify-after-drain.spec.tsx b/packages/web/test/consistency/c18-classify-after-drain.spec.tsx new file mode 100644 index 000000000..8618c0640 --- /dev/null +++ b/packages/web/test/consistency/c18-classify-after-drain.spec.tsx @@ -0,0 +1,253 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + * + * C18 — classification waits for the drain. + * + * "A recordless adopted occurrence is classified (direct-insert vs invoked) + * only after every record the document already holds for the boundary has + * been applied: no sync that runs between the parser's end and the deferred + * drain — the drain's own first `host.apply`, a live op, the live pump's + * catch-up read — may evaluate a render prop as a zero-arg accessor." + * + * Ruling (frames-rulings 3.5, proposed): "an occurrence is classified only + * after every delivered record has drained" — "pending" is the DRAIN's + * state, not the parser's. The #2968 defer's bound was `recordsPending()` = + * parser running or a fragment pending; a record whose data script already + * ran sat in `_$HY.r` until the deferred `drainRecords` moved it into the + * store, and nothing read that gap (contract §Red R9). + * + * Mechanism meant to carry it: frames/src/client.ts + * `adoptBoundary.recordsPending`'s third term — `_$HY.r` holds a key under + * the boundary's `sc:slot::` / `sc:region:.` prefix that is not yet + * in `appliedRecords` — read by frames/src/frame-client.ts `#syncSlots`' + * defer arm. `drainRecords` marks a key applied BEFORE its `host.apply`, so + * the sync that apply runs sees the other delivered records as pending and + * its own as drained. + * + * Observation: the fills here are REAL — `p =>
  • {p.text}{tick()}
    • `, + * the props read unguarded (plus one untracked identification read, as a + * fill's top-level read is otherwise a STRICT_READ diagnostic). Classified + * direct-insert, the render prop is evaluated as a zero-arg accessor inside + * the insert effect: `p.text` is a `TypeError` and the reactive system + * halts (`REACTIVITY_HALTED`). The pin + * asserts the opposite: every occurrence invoked once with its args, the + * server `
  • ` claimed in place, no error, and the page still reactive + * after the `tick` bump. The harness's replay pins (`harness/replay.spec.tsx` + * C18 ×3) hold the same three orders through the oracle's tolerant fill; + * this file pins the consequence for a real one. + */ +import { afterEach, describe, expect, test, vi } from "vitest"; +import { createSignal, flush, untrack } from "solid-js"; +import { hydrate } from "@solidjs/web"; +import { + bootPage, + fillHtml2, + frameHtml, + freshFid, + holeHtml, + microtasks, + quiesce, + slotRange, + type Page +} from "./support.js"; + +let page: Page | undefined; +afterEach(async () => { + await page?.cleanup(); + page = undefined; +}); + +/** The server render of `p =>
  • {p.text}{tick()}
    • ` at tick 0. */ +const liveFillHtml = (fid: string, occ: string, text: string) => fillHtml2(fid, occ, text, "0"); + +/** + * The parser's clock: `document.readyState` reads "loading" until `done()` + * — the records the document still owes execute while it is running, and + * the response's tail (the last data script, then the end) parses in one + * go before any timer fires, so the restore is synchronous with the last + * record. + */ +function parserRunning() { + const spy = vi.spyOn(document, "readyState", "get").mockReturnValue("loading"); + return () => spy.mockRestore(); +} + +describe("C18 — classification waits for the drain", () => { + // Arm (a): the drain's own first apply. Two render-prop occurrences; both + // records owed when the boundary adopts (the #2968 defer arms); both + // execute, the parser finishes, THEN the deferred drain fires. Its first + // `host.apply` syncs the frame while the second record is still one loop + // iteration away in `_$HY.r`: that sync must defer item#1, not classify + // it — the loop's next apply mounts it with its args. + test("(a) two records drained after the parser finished: each occurrence claims with its args; nothing is evaluated argless", async () => { + const fid = freshFid("c18a"); + const parserDone = parserRunning(); + page = bootPage( + frameHtml( + fid, + `
      ${slotRange("item#0", liveFillHtml(fid, "item#0", "p0"))}${slotRange( + "item#1", + liveFillHtml(fid, "item#1", "p1") + )}
    ` + ) + ); + const serverLis = [...page.container.querySelectorAll("li")]; + const Comp = (globalThis as any)._$SC.r(fid); + const [tick, setTick] = createSignal(0); + const invoked: string[] = []; + const dispose = hydrate( + () => ( + { + invoked.push(untrack(() => p.text)); + return ( +
  • + {p.text} + {tick()} +
    • + ); + }} + /> + ), + page.container + ); + // Adopted with the parser running: both occurrences deferred, nothing + // invoked, the server markup untouched. + expect(invoked).toEqual([]); + expect(page.container.textContent).toBe("p0" + "0" + "p1" + "0"); + + // The document's tail: both data scripts, then the end of the response + // — before the deferred drain's macrotask. + page.slotRecord(fid, "item#0", { text: "p0" }); + page.slotRecord(fid, "item#1", { text: "p1" }); + parserDone(); + expect(invoked).toEqual([]); + + await quiesce(); + await quiesce(); + expect(invoked.sort()).toEqual(["p0", "p1"]); + expect([...page.container.querySelectorAll("li")]).toEqual(serverLis); + expect(page.container.textContent).toBe("p0" + "0" + "p1" + "0"); + setTick(1); + flush(); + expect(page.container.textContent).toBe("p0" + "1" + "p1" + "1"); + expect(page.warnings).toEqual([]); + expect(page.errors).toEqual([]); + dispose(); + }); + + // Arm (b): a sync the drain did not trigger, in the same window. One + // occurrence and a live hole; the record executes and the parser finishes + // while the defer is armed; a live hole op then lands — the pump's read is + // a microtask, the drain a macrotask — and its `host.apply` syncs the frame + // over the undrained record. That sync must defer the occurrence; the + // drain's apply mounts it. (The pump's catch-up read over ops logged + // before adoption is the same sync from the other side of adoption — + // `harness/replay.spec.tsx`'s third C18 pin.) + test("(b) a live op syncs the frame between the parser's end and the drain: the occurrence defers, then claims with its args", async () => { + const fid = freshFid("c18b"); + const parserDone = parserRunning(); + page = bootPage( + frameHtml( + fid, + `
      ${slotRange("item#0", liveFillHtml(fid, "item#0", "p0"))}

    ${holeHtml( + 18, + "hole-v0" + )}

    ` + ) + ); + const serverLi = page.container.querySelector("li")!; + const Comp = (globalThis as any)._$SC.r(fid); + const [tick, setTick] = createSignal(0); + const invoked: string[] = []; + const dispose = hydrate( + () => ( + { + invoked.push(untrack(() => p.text)); + return ( +
  • + {p.text} + {tick()} +
    • + ); + }} + /> + ), + page.container + ); + expect(invoked).toEqual([]); + + page.slotRecord(fid, "item#0", { text: "p0" }); + parserDone(); + // The live op's sync lands on the pump's microtask read — before the + // deferred drain's macrotask. + page.live.push({ type: "hole", key: "lh:18", html: "hole-v1" }); + await microtasks(4); + expect(page.container.querySelector("p")!.textContent).toBe("hole-v1"); + expect(invoked).toEqual([]); + + await quiesce(); + await quiesce(); + expect(invoked).toEqual(["p0"]); + expect(page.container.querySelector("li")).toBe(serverLi); + expect(page.container.textContent).toBe("p0" + "0" + "hole-v1"); + setTick(1); + flush(); + expect(page.container.textContent).toBe("p0" + "1" + "hole-v1"); + expect(page.warnings).toEqual([]); + expect(page.errors).toEqual([]); + dispose(); + }); + + // Control: a tick between the two records — the drain runs while the + // parser is still owed the second, every sync reads `recordsPending()` + // true through the parser's term alone. Green before and after 3.5. + test("control: a drain per record while the parser is still running classifies nothing early", async () => { + const fid = freshFid("c18c"); + const parserDone = parserRunning(); + page = bootPage( + frameHtml( + fid, + `
      ${slotRange("item#0", liveFillHtml(fid, "item#0", "p0"))}${slotRange( + "item#1", + liveFillHtml(fid, "item#1", "p1") + )}
    ` + ) + ); + const Comp = (globalThis as any)._$SC.r(fid); + const [tick, setTick] = createSignal(0); + const invoked: string[] = []; + const dispose = hydrate( + () => ( + { + invoked.push(untrack(() => p.text)); + return ( +
  • + {p.text} + {tick()} +
    • + ); + }} + /> + ), + page.container + ); + page.slotRecord(fid, "item#0", { text: "p0" }); + await quiesce(); + expect(invoked).toEqual(["p0"]); + page.slotRecord(fid, "item#1", { text: "p1" }); + parserDone(); + await quiesce(); + await quiesce(); + expect(invoked).toEqual(["p0", "p1"]); + setTick(1); + flush(); + expect(page.container.textContent).toBe("p0" + "1" + "p1" + "1"); + expect(page.warnings).toEqual([]); + expect(page.errors).toEqual([]); + dispose(); + }); +}); diff --git a/packages/web/test/consistency/harness/replay.spec.tsx b/packages/web/test/consistency/harness/replay.spec.tsx index cb41d0c68..1d7a738e0 100644 --- a/packages/web/test/consistency/harness/replay.spec.tsx +++ b/packages/web/test/consistency/harness/replay.spec.tsx @@ -37,60 +37,57 @@ async function findings(scenario: Scenario, id: string) { } describe("harness replay — reduced counterexamples", () => { - // C18 — classification waits for the drain. Observed on `next`: with two - // records owed after adoption and the parser done before the deferred - // drain fires, the drain's FIRST `host.apply` syncs the frame, which - // finds the second occurrence recordless with `recordsPending()` false - // and classifies it direct-insert — the render prop is evaluated as a - // zero-arg accessor (1 zero-arg call). Expected: 0 — a recordless adopted - // occurrence is classified only after every record the document holds - // has been applied. - test.fails( - "C18 classify-after-drain: two records drained after the parser finished — the first apply classifies the second", - async () => { - expect( - await findings( - { ...base, occurrences: [render(0), render(1)], events: [H, R(0), R(1)] }, - "C18" - ) - ).toEqual([]); - } - ); + // C18 — classification waits for the drain (frames-rulings 3.5: "an + // occurrence is classified only after every delivered record has + // drained" — "pending" is the drain's state, not the parser's; contract + // C18, red R9). Was red on `next`: with two records owed after adoption + // and the parser done before the deferred drain fired, the drain's FIRST + // `host.apply` synced the frame, which found the second occurrence + // recordless with `recordsPending()` false — its record one loop + // iteration away in `_$HY.r` — and classified it direct-insert: the + // render prop evaluated as a zero-arg accessor (1 zero-arg call; a real + // fill's props read is a `TypeError` → `REACTIVITY_HALTED`). Carried + // now by `adoptBoundary.recordsPending`'s third term: a delivered record + // not yet in `appliedRecords` is pending, so that sync defers the second + // occurrence and the loop's next apply mounts it with its args. + test("C18 classify-after-drain: two records drained after the parser finished — the first apply defers the second, the second apply mounts it", async () => { + expect( + await findings( + { ...base, occurrences: [render(0), render(1)], events: [H, R(0), R(1)] }, + "C18" + ) + ).toEqual([]); + }); // C18, second trigger: a live hole op arriving between the parser's end - // and the deferred drain syncs the frame; the one record sits undrained - // in `_$HY.r`. Observed: 1 zero-arg call. Expected: 0. - test.fails( - "C18 classify-after-drain: a live op syncs the frame before the deferred drain", - async () => { - expect( - await findings( - { ...base, occurrences: [render(0)], liveHole: true, events: [H, R(0), L("ab"), tick] }, - "C18" - ) - ).toEqual([]); - } - ); + // and the deferred drain syncs the frame while the one record sits + // undrained in `_$HY.r`. That sync reads the record as pending (3.5) and + // defers; the drain's apply mounts the occurrence with its args. + test("C18 classify-after-drain: a live op's sync before the deferred drain defers the undrained occurrence", async () => { + expect( + await findings( + { ...base, occurrences: [render(0)], liveHole: true, events: [H, R(0), L("ab"), tick] }, + "C18" + ) + ).toEqual([]); + }); // C18, third trigger: ops logged before adoption replay through the live // pump's first async read — after the record landed, before the drain. - // Observed: 1 zero-arg call. Expected: 0. - test.fails( - "C18 classify-after-drain: the live pump's catch-up read syncs before the drain", - async () => { - expect( - await findings( - { - ...base, - occurrences: [render(0)], - liveHole: true, - events: [L("ab"), L("cd"), H, R(0), tick] - }, - "C18" - ) - ).toEqual([]); - } - ); + // Same window, same answer: the catch-up sync defers, the drain mounts. + test("C18 classify-after-drain: the live pump's catch-up read before the drain defers the undrained occurrence", async () => { + expect( + await findings( + { + ...base, + occurrences: [render(0)], + liveHole: true, + events: [L("ab"), L("cd"), H, R(0), tick] + }, + "C18" + ) + ).toEqual([]); + }); // C18 control: a tick between the two records lets the drain run while the // parser is still owed the second — each sync finds `recordsPending()` true.