Skip to content

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

Open
yumemi-thomas wants to merge 1 commit into
solidjs:nextfrom
yumemi-thomas:fix/reconcile-proxy-values
Open

fix(reconcile): unwrap store proxies to their raw value in the diff (closes #2825)#2826
yumemi-thomas wants to merge 1 commit into
solidjs:nextfrom
yumemi-thomas:fix/reconcile-proxy-values

Conversation

@yumemi-thomas

Copy link
Copy Markdown

Closes #2825

Summary

reconcile() mishandles values that are store proxies — the normal shape for store-in-store composition ({ selected: row }, lists of row stores, what createProjection commits). Once the nested proxies have live tracked nodes (i.e. they are rendered anywhere), three things go wrong: a keyed swap writes internal signal-node records into the store (bindings render [object Object], JSON.stringify throws on circularity), a keyless swap is silently dropped, and a keyed reorder of an array whose items are proxies crashes synchronously with RangeError: Maximum call stack size exceeded. All three are 1.x → 2.0 regressions.

Repro (one panel per symptom): https://stackblitz.com/edit/solidjs-templates-qjajl3gp?file=src%2FApp.tsx

Root cause

Two related gaps in store/reconcile.ts:

  1. The diff's unwrap helper reads the wrong slot off a proxy: value?.[$TARGET]?.[STORE_NODE] — the per-property signal-node bookkeeping record — instead of STORE_VALUE, the raw data. The object diff then compares, keys, and re-wraps the internal record (symptoms 1–2). It has been this way since the helper was introduced (880c97b3), likely a slip between the adjacent one-letter slots ("n" vs "v"); it survived because the wrong slot is only populated once nested proxies are rendered, which non-rendering tests never do. 1.x deep-unwraps proxies before diffing — this helper appears to be the port of that step, pointed at the wrong slot.

  2. Nothing normalizes next on entry: the array branches (and root reconcile() calls) hand applyState a proxy as next, whose swap then sets a row store's STORE_VALUE to its own proxy — every subsequent read recurses through itself (symptom 3).

Fix

Two lines: unwrap reads STORE_VALUE, and applyState normalizes next = unwrap(next) as its first statement. All next-side routes into the diff (object props, every array branch, recursion, root calls) funnel through that single entry, and only the next side is ever written — the previous side is read-only and proxy-safe by construction (keyFn/keyedMatch read through proxies; wrap() short-circuits on $PROXY).

unwrap deliberately stays a shallow raw read rather than reusing unwrapStoreValue: that helper clones when a STORE_OVERRIDE is pending, and clones break the diff's identity mechanics (previous === next skip checks, wrap(raw) resolving back to the existing proxy via STORE_LOOKUP). Pending overrides are honored at the right layer instead — wrap(raw) returns the same proxy with its overrides, and the recursion dispatches to the override-aware slow path.

Beyond fixing the crash, this restores reconcile's identity contract across the proxy boundary: after a keyed reorder list[0] === rowB holds, and reconciling raw server data into a list of proxies deep-merges into the existing row stores (their independent subscribers update) instead of replacing them.

Solid 1.x note

All three scenarios verified working on solid-js@1.9.14: the keyed swap lands with identity (selected === rowB), the keyless swap applies, and the proxy-array reorder preserves identity on every row. Clean full regression, no caveats.

Tests

Three red-first tests in tests/store/reconcile.test.ts, one per symptom:

  • swapping a tracked row proxy held as a property value (the internals leak)
  • keyless store-proxy property values still swap (the silent drop)
  • reordering an array whose items are store proxies (the stack overflow)

Each confirmed failing on the unfixed source and reproducing against the published solid-js@2.0.0-beta.15 npm package; the crash case additionally asserts row identity is preserved through the reorder. Full solid-signals / solid-js / @solidjs/web suites pass.

Everything before the 1.x note stays as given. The claims in the paragraph are all accurate for this PR — the three tests were red-first (verified on unfixed source), and both the bugs and the fixed behavior were run against published beta.15 (bugs) and the built branch (fixes). Delete the trailing --- + italic line from the previous version so the disclosure appears exactly once.

Full disclosure: I wrote this with the help of an AI assistant (Claude Fable 5) and reviewed every line before pushing. All new tests were written first and confirmed failing on the unfixed code, and the bugs and fixes were both verified against the published beta.15 npm package, not just this repo.

@changeset-bot

changeset-bot Bot commented Jul 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cfe6c8f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@solidjs/signals Patch
test-integration Patch
solid-js Patch
babel-preset-solid Patch
@solidjs/web Patch
@solidjs/html Patch
@solidjs/h Patch
@solidjs/universal Patch
@solidjs/element Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yumemi-thomas yumemi-thomas force-pushed the fix/reconcile-proxy-values branch from 79f4137 to cfe6c8f Compare July 4, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant