fix(signals): guard reconcile's array paths against array/object shape changes

.changeset/reconcile-array-object-shape-guard.md

@@ -0,0 +1,18 @@
+---
+"@solidjs/signals": patch
+---
+
+Stop `reconcile` from merging an array into an object slot (and vice versa) inside arrays
+
+The object diff has always refused to recurse into a pair whose kinds disagree — `Array.isArray(previous) !== Array.isArray(next)` replaces the slot instead of merging. The array paths reach the same recursion through `keyedMatch` and positional pairing, and neither applied that rule: two keyless wrappables "match" because `keyFn` returns `undefined` for both, regardless of whether they are arrays or objects.
+
+The result was a slot whose proxy is permanently the wrong kind. `Array.isArray` on a store proxy inspects the proxy's target, which is fixed at wrap time, so after
+
+```js
+const [state, setState] = createStore({ list: [{ x: 1 }] });
+setState(reconcile({ list: [[10, 20]] }, "id"));
+```
+
+`state.list[0]` reports `Array.isArray === false` and enumerates as `{ "0": 10, "1": 20 }` — spread, `.map`, `<For each>` and `JSON.stringify` all see an object. The reverse direction (array slot receiving an object) is worse: the array-shaped target reads `length` off the incoming object and the store presents an empty array, silently dropping the data.
+
+Four call sites shared the gap — the keyed prefix loop and the positional loop in both `applyStateFast` and `applyStateSlow`, plus `applyArrayItem` (which covers the keyed diff's trailing and moved slots). They now share a `recursablePair()` helper that folds the existing raw-value (`markRaw`) check together with the container-kind check, so a kind change replaces the slot by reference — exactly what the object diff and `descendInto` already do.

packages/solid-signals/src/store/reconcile.ts

@@ -118,6 +118,22 @@ function keyedMatch(a: any, b: any, keyFn: (item: NonNullable<any>) => any) {
   return a === b || (isWrappable(a) && isWrappable(b) && keyFn(a) === keyFn(b));
 }
 
+// A pair of array slots may only be merged into when both sides are real store
+// children of the SAME container kind. An array and an object are different
+// shapes, and merging one into the other leaves the slot's proxy permanently
+// mismatched with its value (an array target holding an object, so
+// `Array.isArray`/spread/`map` lie). The object diff has always applied this
+// rule; the array paths reach the same recursion through `keyedMatch` /
+// positional pairing, where two keyless wrappables "match" regardless of kind.
+function recursablePair(previous: any, next: any): boolean {
+  return (
+    isWrappable(previous) &&
+    isWrappable(next) &&
+    !(rawValuesUsed && (isRawValue(previous) || isRawValue(next))) &&
+    Array.isArray(previous) === Array.isArray(next)
+  );
+}
+
 // Array reconciliation updates the slots it visits, then swaps STORE_VALUE.
 // Previously tracked keys that are absent from `next` still need invalidating,
 // and `in` dependencies should follow the new value's membership. Use
@@ -195,11 +211,7 @@ function applyArrayItem(
   node: any,
   keyFn: (item: NonNullable<any>) => any
 ) {
-  if (
-    isWrappable(next) &&
-    isWrappable(previous) &&
-    !(rawValuesUsed && (isRawValue(previous) || isRawValue(next)))
-  ) {
+  if (recursablePair(previous, next)) {
     const wrapped = wrap(previous, target);
     node && setSignal(node, wrapped);
     applyState(next, wrapped, keyFn);
@@ -421,7 +433,7 @@ function applyStateFast(next: any, target: any, keyFn: (item: NonNullable<any>)
         // the recursion dispatch entirely. Raw-marked values are leaves:
         // replace the slot node instead of recursing.
         if (item !== next[start]) {
-          if (rawValuesUsed && (isRawValue(item) || isRawValue(next[start]))) {
+          if (!recursablePair(item, next[start])) {
             arrayNodes?.[start] && setSignal(arrayNodes[start], wrapValue(next[start], target));
           } else applyStateChild(next[start], item, target, keyFn);
         }
@@ -494,11 +506,7 @@ function applyStateFast(next: any, target: any, keyFn: (item: NonNullable<any>)
     } else if (next.length) {
       for (let i = 0, len = next.length; i < len; i++) {
         const item = previous[i];
-        if (
-          isWrappable(item) &&
-          isWrappable(next[i]) &&
-          !(rawValuesUsed && (isRawValue(item) || isRawValue(next[i])))
-        ) {
+        if (recursablePair(item, next[i])) {
           if (item !== next[i]) applyStateChild(next[i], item, target, keyFn);
         } else {
           if (item !== next[i]) changed = true;
@@ -615,8 +623,8 @@ function applyStateSlow(next: any, target: any, keyFn: (item: NonNullable<any>)
         );
         start++
       ) {
-        if (isWrappable(item) && isWrappable(next[start]) && item !== next[start]) {
-          if (rawValuesUsed && (isRawValue(item) || isRawValue(next[start]))) {
+        if (item !== next[start] && isWrappable(item) && isWrappable(next[start])) {
+          if (!recursablePair(item, next[start])) {
             nodes?.[start] && setSignal(nodes[start], wrapValue(next[start], target));
           } else applyState(next[start], wrap(item, target), keyFn);
         }
@@ -688,11 +696,7 @@ function applyStateSlow(next: any, target: any, keyFn: (item: NonNullable<any>)
     } else if (next.length) {
       for (let i = 0, len = next.length; i < len; i++) {
         const item = getOverrideValue(previous, override, i as any, optOverride);
-        if (
-          isWrappable(item) &&
-          isWrappable(next[i]) &&
-          !(rawValuesUsed && (isRawValue(item) || isRawValue(next[i])))
-        ) {
+        if (recursablePair(item, next[i])) {
           if (item !== next[i]) applyState(next[i], wrap(item, target), keyFn);
         } else {
           if (item !== next[i]) changed = true;

packages/solid-signals/tests/store/reconcile.test.ts

@@ -761,6 +761,72 @@ describe("reconcile without a key (positional merge)", () => {
     expect(state.items[0].v).toBe(9);
   });
 
+  describe("array/object shape changes at an array slot", () => {
+    // Two keyless wrappables "match" for the array diff (both report no key),
+    // so an array and an object used to be merged into each other, leaving the
+    // slot's proxy permanently the wrong kind.
+    test("object -> array at an unkeyed position", () => {
+      const [state, setState] = createStore<any>({ list: [{ x: 1 }] });
+      createRoot(() => {
+        createEffect(
+          () => state.list[0].x,
+          () => {}
+        );
+      });
+      flush();
+      setState(reconcile({ list: [[10, 20]] }, "id"));
+      flush();
+      expect(Array.isArray(state.list[0])).toBe(true);
+      expect(snapshot(state.list[0])).toEqual([10, 20]);
+    });
+
+    test("array -> object at an unkeyed position", () => {
+      const [state, setState] = createStore<any>({ list: [[10, 20]] });
+      createRoot(() => {
+        createEffect(
+          () => state.list[0][0],
+          () => {}
+        );
+      });
+      flush();
+      setState(reconcile({ list: [{ x: 1 }] }, "id"));
+      flush();
+      expect(Array.isArray(state.list[0])).toBe(false);
+      expect(snapshot(state.list[0])).toEqual({ x: 1 });
+    });
+
+    test("keyless slot trailing a keyed array", () => {
+      const [state, setState] = createStore<any>({ list: [{ id: "a" }, { x: 1 }] });
+      createRoot(() => {
+        createEffect(
+          () => state.list[1].x,
+          () => {}
+        );
+      });
+      flush();
+      setState(reconcile({ list: [{ id: "a" }, [10, 20]] }, "id"));
+      flush();
+      expect(Array.isArray(state.list[1])).toBe(true);
+      expect(snapshot(state.list[1])).toEqual([10, 20]);
+    });
+
+    test("positional (key: null) merge keeps the incoming kind", () => {
+      const [state, setState] = createStore<any>({ list: [{ x: 1 }, [1]] });
+      createRoot(() => {
+        createEffect(
+          () => [state.list[0].x, state.list[1][0]],
+          () => {}
+        );
+      });
+      flush();
+      setState(reconcile({ list: [[7], { y: 2 }] }, null));
+      flush();
+      expect(Array.isArray(state.list[0])).toBe(true);
+      expect(Array.isArray(state.list[1])).toBe(false);
+      expect(snapshot(state.list)).toEqual([[7], { y: 2 }]);
+    });
+  });
+
   test("does not enforce root identity", () => {
     const [state, setState] = createStore({ id: 1, v: 2 });
     // keyed (including the "id" default) this throws "different identity"; key: null must merge