diff --git a/.changeset/frames-data-response-scoped.md b/.changeset/frames-data-response-scoped.md new file mode 100644 index 000000000..27153d38f --- /dev/null +++ b/.changeset/frames-data-response-scoped.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +frames: a `data` chunk is under the store's version guard like every other chunk — a superseded response's late data lands nowhere, never in the table that is now the current response's (contract C5 a, b, e). diff --git a/packages/web/frames/src/frame-client.ts b/packages/web/frames/src/frame-client.ts index d711c90c1..55bac57fd 100644 --- a/packages/web/frames/src/frame-client.ts +++ b/packages/web/frames/src/frame-client.ts @@ -796,8 +796,19 @@ export function createFrameHost(options = {}) { } }, apply(chunk) { - // Data payloads are response-scoped; apply immediately, no store needed. + // Data payloads are response-scoped: they go to the data hook, not + // the store — but under the store's version guard like every other + // chunk (frames-rulings 1.2): a `data` chunk of a response the + // address has moved past lands nowhere, never in the table that is + // now the current response's (the transport restamps every chunk + // with its response's version; the integration rotates the table at + // the header and creates it at first use, so the first use must be + // the current response's). if (chunk.type === "data") { + const store = stores.get(chunk.id); + // (`n < undefined` is false: a store with no version yet, or a chunk + // with none, guards nothing.) + if (store && chunk.version < store.version) return; options.applyData && options.applyData(chunk); return; } diff --git a/packages/web/test/consistency/c05-data-response-scoped.spec.tsx b/packages/web/test/consistency/c05-data-response-scoped.spec.tsx index 9e19eb73e..e353a895c 100644 --- a/packages/web/test/consistency/c05-data-response-scoped.spec.tsx +++ b/packages/web/test/consistency/c05-data-response-scoped.spec.tsx @@ -101,103 +101,89 @@ describe("C5 — data is response-scoped", () => { // trails v2's header, v2's record references `{$ref:"1"}`, v2's own data // for "1" arrives later. The fill must never show v1's value. // - // Observed on `next`: the fill mounts with text "old" (v1's value) and - // stays "old" after v2's data ("new") and `complete` arrive — the frame - // never re-resolves a record it already applied. Expected: the record - // waits (ref unresolved) until v2's data, then shows "new". - // Where it goes wrong: client.ts `beginStream` rotates by setting - // `tables.set(address, undefined)` at v2's header, and `tableFor` creates - // the table LAZILY at first use (`ensureTable`) — so v1's late `data` - // chunk, routed by `createFrameHost.apply` straight to `applyData` with - // no version check (the store guard only covers record writes), is the - // first use and lands in the table that is now v2's. The transport - // restamps chunks with the response's version but the data path never - // reads it; nothing associates a `data` chunk with the stream that - // carried it once the header has rotated the address's table. - test.fails( - "(a) v1's late data trails v2's header: v2's {$ref} never resolves to v1's value", - async () => { - const fid = freshFid("c5a"); - const getX = createServerReference(fid); - await sharedHost(); - const { held } = stubHeldFetch([WIRE, WIRE]); - const [v1, v2] = held; - const p1 = getX(1); - const p2 = getX(1); - // Both headers resolved: bump(A)=1 → onStream(A); bump(A)=2 → onStream(A). - expect(await p2).toBe(await p1); - v1.send(start(1)); - v2.send(start(1)); - await pump(1); - // v1's data, AFTER v2's header. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - // v2's record references the same ref id (ids restart per response). - v2.send(slot(1, "1")); - v2.send(html(1)); - await pump(1); - const { container, seen } = mountSite(p1); - await pump(); - // The record's ref is v2's; v2's data has not arrived: the fill waits. - expect(seen).toEqual([]); - expect(container.querySelector("li")).toBeNull(); - // v2's own data lands, then the stream completes. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); - v2.send(complete(1)); - v2.close(); - await pump(); - expect(seen).not.toContain("old"); - expect(container.querySelector("li")!.textContent).toBe("new"); - } - ); + // Was red on `next`: the fill mounted with "old" (v1's value) and stayed + // there — `beginStream` rotated the address's table at v2's header, the + // table was created lazily at first use, and v1's late `data` chunk went + // to `applyData` with no version check, so it was the first use and + // landed in v2's table. Green under frames-rulings 1.2: the data path is + // under the store's version guard like every other chunk — a `data` + // chunk of a response the address has moved past (the response's + // version, restamped by the transport, below the store's) lands nowhere. + test("(a) v1's late data trails v2's header: v2's {$ref} never resolves to v1's value", async () => { + const fid = freshFid("c5a"); + const getX = createServerReference(fid); + await sharedHost(); + const { held } = stubHeldFetch([WIRE, WIRE]); + const [v1, v2] = held; + const p1 = getX(1); + const p2 = getX(1); + // Both headers resolved: bump(A)=1 → onStream(A); bump(A)=2 → onStream(A). + expect(await p2).toBe(await p1); + v1.send(start(1)); + v2.send(start(1)); + await pump(1); + // v1's data, AFTER v2's header. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + // v2's record references the same ref id (ids restart per response). + v2.send(slot(1, "1")); + v2.send(html(1)); + await pump(1); + const { container, seen } = mountSite(p1); + await pump(); + // The record's ref is v2's; v2's data has not arrived: the fill waits. + expect(seen).toEqual([]); + expect(container.querySelector("li")).toBeNull(); + // v2's own data lands, then the stream completes. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); + v2.send(complete(1)); + v2.close(); + await pump(); + expect(seen).not.toContain("old"); + expect(container.querySelector("li")!.textContent).toBe("new"); + }); // Arm (b): the table rotation observed directly through the host's // resolver (what `#refsUnresolved`/`#resolveArgs` call with the frame's // address) — does v1's late data land in the table v2's refs resolve // from, and does it overwrite v2's own value once that has landed? // - // Observed on `next`: after v2's header, `resolve({$ref:"1"}, A)` reads - // "old" from v1's late chunk (expected undefined: v2 has delivered - // nothing); after v2's data ("new") a second late v1 chunk for "1" - // overwrites it to "old" again (expected "new"). Where it goes wrong: - // as in (a) — `tableFor(address)` is one table per ADDRESS at a time, - // keyed by nothing that names the response; `createJSONDataTable.apply` - // sets the key on every `initial` record, so whichever response's chunk - // arrives last owns the key. - test.fails( - "(b) table rotation: a superseded response's late data never lands in the current table", - async () => { - const fid = freshFid("c5b"); - const getX = createServerReference(fid); - const host = await sharedHost(); - const A = frameAddress(fid, [1]); - const { held } = stubHeldFetch([WIRE, WIRE]); - const [v1, v2] = held; - const p1 = getX(1); - const p2 = getX(1); - await p1; - await p2; - v1.send(start(1)); - v2.send(start(1)); - await pump(1); - expect(host.resolve({ $ref: "1" }, A)).toBeUndefined(); - // v1's late data after v2's header. - const v1Data = createDataSource(); - for (const c of v1Data.chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - const afterStaleData = host.resolve({ $ref: "1" }, A); - // v2's data lands. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); - await pump(1); - expect(host.resolve({ $ref: "1" }, A)).toBe("new"); - // Another straggler from v1 (a re-serialized key) after v2's value. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - const afterSecondStale = host.resolve({ $ref: "1" }, A); - expect(afterStaleData).toBeUndefined(); - expect(afterSecondStale).toBe("new"); - } - ); + // Was red on `next`: after v2's header, `resolve({$ref:"1"}, A)` read + // "old" from v1's late chunk, and a second late v1 chunk overwrote v2's + // "new" — one table per ADDRESS at a time, keyed by nothing that named + // the response, every `initial` record setting its key. Green: a stale + // response's data chunks are dropped at the host (see a). + test("(b) table rotation: a superseded response's late data never lands in the current table", async () => { + const fid = freshFid("c5b"); + const getX = createServerReference(fid); + const host = await sharedHost(); + const A = frameAddress(fid, [1]); + const { held } = stubHeldFetch([WIRE, WIRE]); + const [v1, v2] = held; + const p1 = getX(1); + const p2 = getX(1); + await p1; + await p2; + v1.send(start(1)); + v2.send(start(1)); + await pump(1); + expect(host.resolve({ $ref: "1" }, A)).toBeUndefined(); + // v1's late data after v2's header. + const v1Data = createDataSource(); + for (const c of v1Data.chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + const afterStaleData = host.resolve({ $ref: "1" }, A); + // v2's data lands. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); + await pump(1); + expect(host.resolve({ $ref: "1" }, A)).toBe("new"); + // Another straggler from v1 (a re-serialized key) after v2's value. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + const afterSecondStale = host.resolve({ $ref: "1" }, A); + expect(afterStaleData).toBeUndefined(); + expect(afterSecondStale).toBe("new"); + }); // Arm (c) (control): the normal order — v1 is complete before v2's header. // v1's data went to v1's table; v2's header rotates; v2's record waits for @@ -250,88 +236,84 @@ describe("C5 — data is response-scoped", () => { // stream. Then B-v1's late data for "1" lands, B-v2's record references // "1", B-v2's own data for "1" comes last. // - // Observed on `next`: the fill mounts with "old" (B-v1's value) under - // B-v2's record and stays "old" after B-v2's data. Expected: pending until - // B-v2's data, then "new". Where it goes wrong: as in (a) — the rotation - // at `onStream` is per address and the data path has no version. - test.fails( - "(e) through dynamic: A → B → A → B while B's first body is open; B-v1's late data never answers B-v2's record", - async () => { - const fid = freshFid("c5e"); - const getX = createServerReference(fid); - await sharedHost(); - // fetch order: A(v1), B(v1), A(v2), B(v2) - const { held, calls } = stubHeldFetch([WIRE, WIRE, WIRE, WIRE]); - const [a1, b1, a2, b2] = held; - const [n, setN] = createSignal(1); - const Site = dynamic(() => getX(n()) as any); - const seen: string[] = []; - let div!: HTMLDivElement; - const dispose = createRoot(d => { -