diff --git a/.changeset/fix-reconcile-proxy-values.md b/.changeset/fix-reconcile-proxy-values.md new file mode 100644 index 000000000..f11866936 --- /dev/null +++ b/.changeset/fix-reconcile-proxy-values.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Fix `reconcile()` mishandling store proxies held as values (#2825). The diff's `unwrap` helper read the internal signal-node record (`STORE_NODE`) instead of the store's value, so reconciling data containing store proxies leaked internal Signal records into store reads (breaking `JSON.stringify` with a circular-structure error) or silently dropped keyless swaps; keyed reorder of an array whose items are store proxies crashed with a stack overflow. Proxies are now normalized to their raw value at the diff entry, restoring identity-preserving merges across the proxy boundary. diff --git a/packages/solid-signals/src/store/reconcile.ts b/packages/solid-signals/src/store/reconcile.ts index f0709b806..e616fe575 100644 --- a/packages/solid-signals/src/store/reconcile.ts +++ b/packages/solid-signals/src/store/reconcile.ts @@ -17,7 +17,7 @@ import { } from "./store.js"; function unwrap(value: any) { - return value?.[$TARGET]?.[STORE_NODE] ?? value; + return value?.[$TARGET]?.[STORE_VALUE] ?? value; } function getOverrideValue(value: any, override: any, key: string, optOverride?: any) { @@ -68,6 +68,9 @@ function applyArrayItem( // branches on a `fastPath` boolean, so V8 sees a tighter, more inlinable // shape for the overwhelmingly common case of plain stores. function applyState(next: any, state: any, keyFn: (item: NonNullable) => any) { + // Array items and root calls can pass a store proxy as `next`; normalize to + // its raw value or the swap would set a store's STORE_VALUE to its own proxy. + next = unwrap(next); const target = state?.[$TARGET]; if (!target) return; if (target[STORE_OVERRIDE] || target[STORE_OPTIMISTIC_OVERRIDE]) { diff --git a/packages/solid-signals/tests/store/reconcile.test.ts b/packages/solid-signals/tests/store/reconcile.test.ts index 1ccf14f6e..eaf30ddec 100644 --- a/packages/solid-signals/tests/store/reconcile.test.ts +++ b/packages/solid-signals/tests/store/reconcile.test.ts @@ -227,6 +227,122 @@ describe("setState with reconcile", () => { setState(reconcile([5], "id")); expect(snapshot(state)).toEqual([5]); }); + test("Reconcile swaps a property whose value is another store's proxy", () => { + type Row = { id: number; name: string }; + const [rowA] = createStore({ id: 1, name: "a" }); + const [rowB] = createStore({ id: 2, name: "b" }); + + // rows rendered somewhere -> live tracked nodes + let effA: Row | undefined; + let effB: Row | undefined; + createRoot(() => { + createEffect( + () => ({ id: rowA.id, name: rowA.name }), + v => { + effA = v; + } + ); + createEffect( + () => ({ id: rowB.id, name: rowB.name }), + v => { + effB = v; + } + ); + }); + flush(); + expect(effA).toEqual({ id: 1, name: "a" }); + expect(effB).toEqual({ id: 2, name: "b" }); + + const [state, setState] = createStore<{ selected: Row }>({ selected: rowA }); + let selectedName: string | undefined; + createRoot(() => { + createEffect( + () => state.selected.name, + v => { + selectedName = v; + } + ); + }); + flush(); + expect(selectedName).toBe("a"); + + setState(reconcile({ selected: rowB }, "id")); + flush(); + + expect(state.selected.id).toBe(2); + expect(state.selected.name).toBe("b"); + expect(selectedName).toBe("b"); + }); + + test("Reconcile reorders an array whose items are store proxies", () => { + type Row = { id: number; name: string }; + const [rowA] = createStore({ id: 1, name: "a" }); + const [rowB] = createStore({ id: 2, name: "b" }); + let names: string | undefined; + createRoot(() => { + // rows rendered somewhere -> live tracked nodes + createEffect( + () => rowA.name + rowB.name, + () => {} + ); + }); + flush(); + + const [list, setList] = createStore([rowA, rowB]); + createRoot(() => { + createEffect( + () => list.map(r => r?.name).join(","), + v => { + names = v; + } + ); + }); + flush(); + expect(names).toBe("a,b"); + + setList(reconcile([rowB, rowA], "id")); + flush(); + + expect(names).toBe("b,a"); + expect(list[0].id).toBe(2); + expect(list[1].id).toBe(1); + expect(snapshot(list)).toEqual([ + { id: 2, name: "b" }, + { id: 1, name: "a" } + ]); + }); + + test("Reconcile swaps keyless store-proxy property values", () => { + type Row = { name: string }; + const [rowA] = createStore({ name: "a" }); + const [rowB] = createStore({ name: "b" }); + createRoot(() => { + createEffect( + () => rowA.name + rowB.name, + () => {} + ); + }); + flush(); + + const [state, setState] = createStore<{ selected: Row }>({ selected: rowA }); + let selectedName: string | undefined; + createRoot(() => { + createEffect( + () => state.selected.name, + v => { + selectedName = v; + } + ); + }); + flush(); + expect(selectedName).toBe("a"); + + setState(reconcile({ selected: rowB }, "id")); + flush(); + + expect(state.selected.name).toBe("b"); + expect(selectedName).toBe("b"); + }); }); // type tests