Skip to content

fix(server): reset sync memo child state on re-pull so hydration keys stay aligned#2827

Closed
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/ssr-sync-memo-child-id-leak
Closed

fix(server): reset sync memo child state on re-pull so hydration keys stay aligned#2827
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/ssr-sync-memo-child-id-leak

Conversation

@brenelz

@brenelz brenelz commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Fixes bug 2 of #2801 ({data().value && <h4>{data().value}</h4>} before a <For> breaking hydration).

Root cause

Hydration keys are owner-path child-id slots. On the client, every recompute resets the owner's child state (dispose + _childCount = 0). The server's lean sync memo — what compiler-emitted _$memo() expression wrappers use — re-runs its compute on every pull after a NotReadyError, but did so without resetting child state, so each failed pull permanently leaked the child-id slots it consumed.

The && shape is uniquely exposed because it compiles to an inner condition memo created inside the outer compute body. With the condition reading a pending async memo in a streamed <Loading>, the server needed three pulls (eager create → pending, discovery read → pending, resolved → success), leaking two slots. The successful pull emitted <h4 _hk=10003> while the client's single successful compute claimed 10001 — nobody matched, so the h4 went unclaimed (duplicated in prod; dev warns "unclaimed server-rendered node"). Sync conditions pull once and are unaffected, and <Show> avoids it via its own already-aligned owner scaffolding — matching the symptoms reported in the issue.

Fix

createSyncMemo.pull() now disposes children and resets _childCount before re-running the compute, mirroring the client's owner reset on recompute, so every pull emits the same ids. First pulls skip the dispose branch (_firstChild/_disposal are null), so the hot path pays one null check. This follows the same server-matches-client slot-alignment pattern as 4cab248 and 1122d74, and also covers the equivalent latent leak when a mapArray/repeat item throws NotReadyError mid-loop.

Tests

  • Server streaming test asserting the emitted keys for the async &&-before-<For> shape (<h4 _hk=10001>, previously 10003).
  • jsdom round-trip test hydrating captured stream output for both the async (streamed fragment) and sync condition shapes, asserting full claim with no unclaimed-node/mismatch warnings and in-place claiming of server nodes.

Suites: solid-web server 116/116, solid-web client 299/299, solid-web hydration 23/23*, packages/solid 417/417.

*Known follow-up, not addressed here: the non-sync server createMemo.update() retry path has the same theoretical leak but needs a children-only reset (a naive disposeOwner would fire the memo's own cleanup and kill the retry); no failing case observed. Bug 1 of #2801 (ghost node after refresh()) is a separate dom-expressions insert() issue and will be a PR there.

🤖 Generated with Claude Code

… stay aligned (solidjs#2801)

Hydration keys are owner-path child-id slots. The server's lean sync memo
(used for compiler-emitted expression wrappers) re-runs its compute on every
pull after a NotReadyError, but did so without resetting the owner's child
state — each failed pull leaked the child-id slots it consumed. With
`{data().value && <h4>...</h4>}` reading a pending async memo inside a
streamed <Loading>, the eager and discovery pulls leaked two slots, so the
successful pull emitted <h4 _hk=10003> while the client's single successful
compute claimed 10001 — the node went unclaimed (duplicated in prod, dev
warned "unclaimed server-rendered node").

The sync memo now disposes children and resets `_childCount` before each
re-pull, mirroring how the client resets an owner on recompute, making every
pull emit the same ids. First pulls skip the dispose branch so the hot path
pays only a null check.

Adds a server streaming test asserting the emitted keys and a jsdom
round-trip test hydrating captured stream output for both the async and sync
condition shapes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 757fa12

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

This PR includes changesets to release 9 packages
Name Type
solid-js Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
@solidjs/universal Patch
@solidjs/web Patch
test-integration Patch
babel-preset-solid Patch
@solidjs/signals 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

@codspeed-hq

codspeed-hq Bot commented Jul 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 120 untouched benchmarks


Comparing brenelz:fix/ssr-sync-memo-child-id-leak (757fa12) with next (bad6662)

Open in CodSpeed

ryansolid added a commit that referenced this pull request Jul 6, 2026
…2827's within-hole sync-memo slot leak (knownFailure)

Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid pushed a commit that referenced this pull request Jul 6, 2026
… stay aligned

The server's lean sync memo re-runs its compute on every pull after a
NotReadyError, but did so without resetting the owner's child state, so
each failed pull leaked the child-id slots it consumed (e.g. the
compiler-emitted inner memo of `{cond && <jsx>}`). The eventual
successful pull then emitted hydration keys drifted ahead of the
client's single compute and the nodes went unclaimed (#2801 bug 2).

Mirror the client's owner reset on recompute: dispose children and
reset _childCount before re-running the compute, so every pull emits
the same ids. First pulls skip the dispose branch entirely.

Flips the async-cond-before-for parity-harness scenario from known
failure to passing, and adds a sync-cond-before-for control scenario.

Closes #2827

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member

Landed on next as 1458907 with you as commit author — the runtime fix, server stream test, and changeset are verbatim. The jsdom spec was adapted into the compiled hydration parity harness (which regenerates server artifacts from the live compiler) instead of hand-captured stream HTML: the async-cond-before-for scenario flipped from known-failure to passing, plus a sync control scenario. Thanks for the thorough root-cause work — the within-hole slot-leak diagnosis was exactly right.

@ryansolid ryansolid closed this Jul 6, 2026
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.

2 participants