fix(web): decide SSR text separators on resolved values - #3394
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 98f9d88 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
Coverage Report for CI Build 34729776142Coverage remained the same at 71.842%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
`resolveSSRNode` and `tryResolveString` emitted `<!--!$-->` between two adjacent array items whenever both had a non-object static type — which counted every function. Two neighbouring memos or components were separated as if they were text even when both produced elements, so a list of `Dynamic`, `Show`, or wrapper-library instances paid eight bytes and a comment node per item, and the client removed each one on hydrate. The client claims a multi-insert's nodes positionally after flattening its value, so what matters is what each item RESOLVES to: two texts must land in distinct text nodes; elements and template markup never need a separator. Track "last appended was text" through the walk (`ssrTextTail`, maintained at the leaves; arrays and functions pass it through; an unresolved async hole still counts as text on both sides) and reset it per independent region via the exported wrappers. Deciding on the resolved value also separates text the old rule merged: across a dropped nullish/boolean item (`[7, null, "x"]` → `7x`, one node) and across a nested array boundary (`["a", ["b"]]` → `ab`). Both were latent hydration mismatches — the second text item claimed nothing and materialized a client node on the first update. Parity harness: five scenarios covering the lists, the null gap, nested arrays, and mixed memo results, plus two invariants — `adoptAll` (every node is a server node after hydration AND after the update pass; a failed text claim is invisible to textContent until the update) and `noSeparators`. Against the previous walker, the server side emits separators for both element lists and the hydrate side reports `client-created text "x"` / `"b"` after update. Co-authored-by: Cursor <cursoragent@cursor.com>
test-types failed on next's Component type (SolidElement return): the #3383 fixtures return memos, valid children at runtime but not in the type. Cast at the three sites, JSX imported from @solidjs/web. Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
force-pushed
the
fix/ssr-text-separators
branch
from
September 13, 2026 01:06
98280fc to
92715ad
Compare
ryansolid
added a commit
that referenced
this pull request
Sep 14, 2026
`Show`'s `fallback` prop is typed as an element; the zero-arg thunk is a runtime-accepted shape the scenario exercises on purpose, so cast it (the same treatment as the memo-returning components in #3394). Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
added a commit
that referenced
this pull request
Sep 14, 2026
…client's flatten computed (#3414) (#3428) * fix(solid): resolve SSR boundary children in a scope mirroring the client's flatten computed (#3414) A server-rendered `<Errored>` fallback hydrated dead when the boundary's children threw synchronously and the fallback was a zero-arg thunk (`fallback={() => <Fallback />}`) inside an enclosing boundary. Both sides hand the thunk back unresolved and the enclosing boundary unwraps it — the client inside its second computed (`boundaryComputed(() => flatten(read(c)))`, the boundary owner's child `…1`), the server inline under the boundary owner (`ctx.resolve` right after the `fn` owner `…0`) — so the fallback's root element took `…1` on the server and `…10` on the client, failed its claim, and its handlers and effects never attached. Same misalignment for `Loading` as the consumer, fragment children, and `Show`'s fallback thunk; an element hole in between (compiled `scope()` ↔ transparent insert effect) was already aligned. The server `Errored` and `Loading` boundaries now resolve their children's result in a virtual id scope mirroring that second computed — ssrScope's technique: the owner keeps its identity (retry wraps capture it and read the current pull's error handler off it; a real child owner snapshots a stale handler and double-renders the fallback) and only its id counter is rewritten for the resolve, retries continuing where the discovery pass left it. The error record is serialized at the boundary's own id, read once, since `owner.id` is rewritten while the error lands. Parity-harness scenarios cover Errored/Loading/fragment consumers and the Show producer, plus the element-hole control. Closes #3414 Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> * test(web): type the Show fallback thunk scenario as an element `Show`'s `fallback` prop is typed as an element; the zero-arg thunk is a runtime-accepted shape the scenario exercises on purpose, so cast it (the same treatment as the memo-returning components in #3394). Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
added a commit
that referenced
this pull request
Sep 14, 2026
#3394 split `tryResolveString` and `resolveSSRNode` into an entry function that resets `ssrTextTail` and a recursive body. The bodies are too big for V8 to inline, so every template hole paid one extra call — about 2% of SSR throughput on element-heavy pages, up to 7% where holes are dense. Pass a `nested` flag through the recursion instead: entry calls (no flag) reset the separator state, recursive calls keep it. One function each, identical output. `server-mock.ts` mirrors the signature. Co-authored-by: Cursor <cursoragent@cursor.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3383.
Problem
resolveSSRNodeandtryResolveStringemitted<!--!$-->between two adjacent array items whenever both had a non-object static type — which counted every function. Two neighbouring memos or components were separated as if they were text even when both produced elements:Every
Dynamic,Show, or wrapper-library instance in a list paid eight bytes and a comment node, and the clientnode.remove()d each one duringstripTextSeparators.Fix
The client claims a multi-insert's nodes positionally after flattening the value (memos resolved, nested arrays spliced, nullish dropped), so what matters is what each item resolves to: two texts must land in distinct text nodes; elements and template markup never need a separator. The walkers now track "last appended was text" (
ssrTextTail), maintained at the leaves — arrays and functions pass it through, an unresolved async hole still counts as text on both sides — and reset it per independent region (root resolve, template hole, element children) via the exported wrappers. No function is invoked more than before; hydration id order is unchanged. Thetoprule forssrElement's direct children is preserved as-is.Deciding on the resolved value also separates text the old rule merged, both latent hydration mismatches:
[7, null, "x"]rendered7x(one node)["a", ["b"]]renderedabIn both, the second text item claimed nothing and materialized a client node on the first update.
Tests
test/server/ssr-text-separators.spec.tsx— 8 cases (element lists, Dynamic lists, text/text, null/undefined/false/true gaps, nested arrays, mixed memo results, 200-row list asserting zero separators).scenarios.tsx, real compiler on both sides, streamed artifact → jsdom hydrate → update pass): fiveseparator-*scenarios and two new invariants —adoptAll(every node in the container is a server node after hydration and after the update pass; a failed text claim reads correctly viatextContentuntil the update) andnoSeparators.escape-late-values.spec.tsxthat had encoded the merged7xoutput.Against the previous walker, same scenarios: the server side fails
noSeparatorsfor both element lists, and the hydrate side reportsclient-created text "x" after update(text-after-null) andclient-created text "b" after update(nested-array-text). Two existing artifacts lose a spurious leading separator (a client-only component'sundefinedreturn counted as text).@solidjs/webdom 77/734, server 85/804, hydrate 28/173 green. Neither compiler emits the separator; runtime-only. Independent of #3393 (whitespace-tolerant assertions where the two touch).Surfaced by the
@yak/solidaudit (DigitecGalaxus/next-yak#644): the base runtime rendered every styled element throughDynamic, so every list carried one of these per element.Authored with Claude via Cursor.