diff --git a/.changeset/fix-optimistic-removed-prop-read.md b/.changeset/fix-optimistic-removed-prop-read.md new file mode 100644 index 000000000..6f92f4c68 --- /dev/null +++ b/.changeset/fix-optimistic-removed-prop-read.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +An optimistic store over a store reads a property the inner store's reconcile removed as `undefined` instead of its last value (#3883) diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index 8883c70e6..8687e305a 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -390,7 +390,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | §6c | live | `docs/INTERNALS-STORE-STATE.md:334` | — | createProjection.async.test.ts×1 flight-owned-transaction.test.ts×1 | Store-wide status gating (RUL-7) | | §6d | live | `docs/INTERNALS-STORE-STATE.md:347` | reconcile.ts×2 target.ts×2 | — | Diff reachability (RUL-11) | | §7 | live | `docs/INTERNALS-STORE-STATE.md:359` | optimistic.ts×1 projection.ts×1 | — | Projections & optimism layering | -| §7b | live | `docs/INTERNALS-STORE-STATE.md:369` | scheduler.ts×1 affects.ts×1 optimistic.ts×2 projection.ts×1 reconcile.ts×1 store.ts×7 target.ts×4 | optimistic-chained-revert-3672-memo.test.ts×1 | Chained backing (cross-store) — spec | +| §7b | live | `docs/INTERNALS-STORE-STATE.md:369` | scheduler.ts×1 affects.ts×1 optimistic.ts×2 projection.ts×1 reconcile.ts×1 store.ts×8 target.ts×4 | optimistic-chained-revert-3672-memo.test.ts×1 optimistic-removed-prop-3883.test.ts×1 | Chained backing (cross-store) — spec | | §8 | live | `docs/INTERNALS-STORE-STATE.md:440` | — | l2-contract.test.ts×1 reconcile-resend-identity.test.ts×1 | Assumptions / open questions | | §8b | live | `docs/INTERNALS-STORE-STATE.md:496` | — | — | Suite-mined rules (2026-08-16) — index & rulings needed | | §9 | live | `docs/INTERNALS-STORE-STATE.md:732` | — | — | Decision log | diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index b4cb97bf0..f23eaf260 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -2002,6 +2002,9 @@ const traps: ProxyHandler = { // Inherited: prototype getters/methods run with the proxy receiver. v = Reflect.get(src, key, receiver); if (typeof v === "function") return v; // proto methods untracked + // Read-through (§7b): the inner store answers an absent key too — the + // outer's node is a subscription point unless it carries a guess. + if (target.ch && src === target.v) return serveDataKey(target, key, v, src, node0, accProbe); // Reading a currently-absent own key subscribes to it (R12) — for any // target OUTSIDE its own draft scope, even mid-setter (#3037, above). if (v === undefined && !inDraft(target)) { diff --git a/packages/signals/tests/store/optimistic-removed-prop-3883.test.ts b/packages/signals/tests/store/optimistic-removed-prop-3883.test.ts new file mode 100644 index 000000000..60a5ea243 --- /dev/null +++ b/packages/signals/tests/store/optimistic-removed-prop-3883.test.ts @@ -0,0 +1,203 @@ +/** + * #3883 — an optimistic store over a store: a property the inner store's + * reconcile removed kept reading its last value through the view. The view's + * node for the key is a subscription point (§7b); the absent-key read served + * its cached value instead of reading through to the inner store. + */ +import { describe, expect, it } from "vitest"; +import { + action, + createOptimisticStore, + createRenderEffect, + createRoot, + createSignal, + createStore, + flush +} from "../../src/index.js"; + +type Card = { id: string; createFailed?: boolean }; + +function setup(optimistic = true) { + const [source, setSource] = createSignal([{ id: "A", createFailed: true }]); + let local!: Card[]; + let setLocal!: (fn: (draft: Card[]) => Card[] | void) => void; + let cards!: Card[]; + let setCards!: (fn: (draft: Card[]) => Card[] | void) => void; + const seen: (boolean | undefined)[] = []; + const dispose = createRoot(d => { + [local, setLocal] = createStore(() => source(), []); + if (optimistic) [cards, setCards] = createOptimisticStore(local); + else cards = local; + createRenderEffect( + () => cards[0]?.createFailed, + value => void seen.push(value) + ); + return d; + }); + flush(); + return { source, setSource, local, setLocal, cards: () => cards, setCards, seen, dispose }; +} + +describe("#3883 a property reconciliation removes", () => { + it("reads undefined through the optimistic view", () => { + const { setSource, local, cards, seen, dispose } = setup(); + expect(cards()[0].createFailed).toBe(true); + + setSource([{ id: "A" }]); + flush(); + expect(local[0].createFailed).toBeUndefined(); + expect(cards()[0].createFailed).toBeUndefined(); + expect(seen).toEqual([true, undefined]); + dispose(); + }); + + it("reads undefined through the derived store with no optimistic layer", () => { + const { setSource, cards, seen, dispose } = setup(false); + setSource([{ id: "A" }]); + flush(); + expect(cards()[0].createFailed).toBeUndefined(); + expect(seen).toEqual([true, undefined]); + dispose(); + }); + + it("reads the re-added value after a remove", () => { + const { setSource, cards, seen, dispose } = setup(); + setSource([{ id: "A" }]); + flush(); + expect(cards()[0].createFailed).toBeUndefined(); + + setSource([{ id: "A", createFailed: false }]); + flush(); + expect(cards()[0].createFailed).toBe(false); + + setSource([{ id: "A" }]); + flush(); + expect(cards()[0].createFailed).toBeUndefined(); + + setSource([{ id: "A", createFailed: true }]); + flush(); + expect(cards()[0].createFailed).toBe(true); + expect(seen).toEqual([true, undefined, false, undefined, true]); + dispose(); + }); + + it("shows a guess on the removed key during an action, undefined after", async () => { + const { setSource, setCards, cards, seen, dispose } = setup(); + let release!: () => void; + const done = action(function* () { + setCards(draft => { + draft[0].createFailed = false; + }); + yield new Promise(resolve => (release = resolve)); + })(); + flush(); + expect(cards()[0].createFailed).toBe(false); + + setSource([{ id: "A" }]); + flush(); + expect(cards()[0].createFailed).toBe(false); + expect(seen.at(-1)).toBe(false); + + release(); + await done; + flush(); + expect(cards()[0].createFailed).toBeUndefined(); + expect(seen.at(-1)).toBeUndefined(); + + const done2 = action(function* () { + setCards(draft => { + draft[0].createFailed = false; + }); + yield new Promise(resolve => (release = resolve)); + })(); + flush(); + expect(cards()[0].createFailed).toBe(false); + expect(seen.at(-1)).toBe(false); + + release(); + await done2; + flush(); + expect(cards()[0].createFailed).toBeUndefined(); + expect(seen.at(-1)).toBeUndefined(); + dispose(); + }); + + it("answers `in` and Object.keys through the view", () => { + const { setSource, cards, dispose } = setup(); + const has: boolean[] = []; + const keys: string[][] = []; + const disposeReaders = createRoot(d => { + createRenderEffect( + () => "createFailed" in cards()[0], + v => void has.push(v) + ); + createRenderEffect( + () => Object.keys(cards()[0]), + v => void keys.push(v) + ); + return d; + }); + flush(); + expect("createFailed" in cards()[0]).toBe(true); + + setSource([{ id: "A" }]); + flush(); + expect("createFailed" in cards()[0]).toBe(false); + expect(Object.keys(cards()[0])).toEqual(["id"]); + expect(has).toEqual([true, false]); + expect(keys).toEqual([["id", "createFailed"], ["id"]]); + disposeReaders(); + dispose(); + }); + + it("reads undefined after an action retains the row and settles (the report)", async () => { + const [source, setSource] = createSignal([]); + let cards!: Card[]; + let setCards!: (fn: (draft: Card[]) => Card[] | void) => void; + let setLocal!: (fn: (draft: Card[]) => Card[] | void) => void; + let release!: () => void; + let create!: () => Promise; + const seen: (boolean | undefined)[] = []; + const dispose = createRoot(d => { + let local: Card[]; + [local, setLocal] = createStore(draft => { + const rows = source(); + const retained = draft.filter(row => row.createFailed && !rows.some(r => r.id === row.id)); + return [...rows, ...retained.map(row => ({ ...row }))]; + }, []); + [cards, setCards] = createOptimisticStore(local); + createRenderEffect( + () => cards[0]?.createFailed, + value => void seen.push(value) + ); + create = action(function* () { + setCards(draft => { + draft.push({ id: "A" }); + }); + yield new Promise(resolve => (release = resolve)); + setLocal(draft => { + draft.push({ id: "A", createFailed: true }); + }); + }); + return d; + }); + flush(); + + const done = create(); + flush(); + expect(cards.map(c => c.id)).toEqual(["A"]); + expect(cards[0].createFailed).toBeUndefined(); + + release(); + await done; + flush(); + expect(cards[0].createFailed).toBe(true); + + setSource([{ id: "A" }]); + flush(); + expect(cards.map(c => c.id)).toEqual(["A"]); + expect(cards[0].createFailed).toBeUndefined(); + expect(seen.at(-1)).toBeUndefined(); + dispose(); + }); +}); diff --git a/scripts/size/scenarios.js b/scripts/size/scenarios.js index b58934052..09deea08e 100644 --- a/scripts/size/scenarios.js +++ b/scripts/size/scenarios.js @@ -992,8 +992,16 @@ module.exports = [ // into it while it is live (`liveTx`, `holdNode`). Cap set at measured + 10 // B rounded up to 0.01 KB. Accepted by the maintainer 2026-10-07 on the // condition hello world stays under 10 KB. - limit: "14.68 KB", - capMinified: 44582, + // Size-Exception (an optimistic view reads a property its inner store + // removed as undefined, #3883, 2026-10-07): 14.68 KB -> 14.70 KB, measured + // at 14,687 B by CI (Size run 37704068417) against `next` @ 3086f1b77's + // 14,671 (+16 B; 7 B over the cap; +40 B minified, 44,597 -> 44,637; + // recorded 44,582 -> 44,637) — the store `get` trap: an absent key on a + // chained target reads through to the inner store (`serveDataKey`). Cap + // set at measured + 10 B rounded up to 0.01 KB. Accepted by the maintainer + // 2026-10-07: store scenarios only, hello world unchanged. + limit: "14.70 KB", + capMinified: 44637, alias }, { @@ -3435,8 +3443,16 @@ module.exports = [ // into it while it is live (`liveTx`, `holdNode`). Cap set at measured + 10 // B rounded up to 0.01 KB. Accepted by the maintainer 2026-10-07 on the // condition hello world stays under 10 KB. - limit: "31.12 KB", - capMinified: 99617, + // Size-Exception (an optimistic view reads a property its inner store + // removed as undefined, #3883, 2026-10-07): 31.12 KB -> 31.17 KB, measured + // at 31,155 B by CI (Size run 37704068417) against `next` @ 3086f1b77's + // 31,155 (+0 B; 35 B over the cap; +40 B minified, 99,632 -> 99,672; + // recorded 99,617 -> 99,672) — the store `get` trap: an absent key on a + // chained target reads through to the inner store (`serveDataKey`). Cap + // set at measured + 10 B rounded up to 0.01 KB. Accepted by the maintainer + // 2026-10-07: store scenarios only, hello world unchanged. + limit: "31.17 KB", + capMinified: 99672, alias }, {