fix(reconcile): unwrap store proxies to their raw value in the diff (closes #2825)

.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.

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>) => 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]) {

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<Row>({ id: 1, name: "a" });
+    const [rowB] = createStore<Row>({ 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<Row>({ id: 1, name: "a" });
+    const [rowB] = createStore<Row>({ 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<Row[]>([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<Row>({ name: "a" });
+    const [rowB] = createStore<Row>({ 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