Repository navigation
fix(ssr): a shell suspension above <Errored> keeps its <Loading> content and its owners - #3770
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 9408b9d The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
Size (brotli, eager entry chunk)
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in |
Coverage Report for CI Build 37285561682Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 75.991%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will degrade performance by 6.15%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | memo + sync render effect only (reference) |
27.8 ms | 29.6 ms | -6.15% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/ssr-shell-suspend-retry (9408b9d) with next (6be6c51)
ryansolid
force-pushed
the
fix/ssr-shell-suspend-retry
branch
from
October 5, 2026 08:24
39c7f3c to
7ef46ec
Compare
…ent and its owners
Two failures when the shell suspends inside an <Errored> subtree that is
not under a <Loading> (the router's flash-decode read after a no-JS post,
a lazy() route with no <Loading> between):
- <Errored> holds its children, placeholders included, in its retry
state, so the whole subtree is a pending root hole. A <Loading> below
it whose content settled first inlined into a shell that did not yet
hold its placeholder: the replace was a no-op, the fragment left the
registry, and the fallback shipped with nothing to swap it. Fragments
settling in that window are now parked and spliced in when a root-hole
re-pull lands their placeholder.
- ssrScope unwraps accessor chains in place. When the accessor that
suspended was returned by the hole's expression ({props.children}
resolving to the <Errored>), the whole scope thunk became the retry,
so every pass re-read the expression and re-created every component
above the boundary; one owning its pending source never converged.
The retry now resumes the accessor that suspended, with the scope's
counter where it was.
Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
force-pushed
the
fix/ssr-shell-suspend-retry
branch
from
October 5, 2026 08:44
7ef46ec to
9408b9d
Compare
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.
Summary
Two SSR failures when the shell suspends inside an
<Errored>subtree that no<Loading>covers. Found through the router's flash-decode read on the page after a no-JS form post (the todos-server example lost its list on that page); alazy()route under<Errored>with no<Loading>in between triggers the second for every request.1. A
<Loading>below the boundary lost its content (@solidjs/web)<Errored>holds its children — placeholders included — in its retry state, so the whole subtree is one pending root hole and the<Loading>placeholder is not in the shellhtmlyet. When the<Loading>content settled before the shell's suspension did, its pre-flush inline (replacePlaceholder(html, key, value)) found no placeholder, the fragment left the registry, and when the root hole was spliced in later it brought the fallback with nothing left to swap it._frstill resolvedtrue.Fix: a fragment settling pre-flush while root holes are pending and its placeholder is absent is parked;
resolveRootHolessplices parked markup in once a re-pull lands the placeholder.2. Every component above the boundary re-rendered per retry pass (
solid-js)ssrScope(the per-hole id scope) runs the hole's expression and then unwraps accessor chains in place. Under a layout's{props.children}hole, the expression resolves to the<Errored>accessor, which throws the suspension — so the whole scope thunk became the retry, and each pass re-readprops.children, re-creatingApp, the router, and everything else above the boundary. A component that owns its pending source started a fresh one per pass and the render never finished. (The router's flash code is already hardened against being re-created mid-request, which looks like this bug seen from the other side.)Fix: when an accessor reached by the unwrap suspends, the scope remembers it and the retry resumes from it, with the scope's id counter restored to where it was. A suspension thrown by the expression itself still re-runs it, as before. This matches the client, which reads the expression once and unwraps in an inner effect.
Public API changes
None. Behaviour change: after a suspension, a scoped hole's retry no longer re-runs the hole's expression when what suspended was an accessor that expression returned.
Tests
test/server/shell-suspend-loading-inline.spec.tsx— a server-component answer and a plain async memo under<Loading>, below a shell suspension, with/without<Errored>and<Document>: content present, fallback gone, nothing rendered twice. The four<Errored>cases fail onnext.test/server/errored-sync-suspend-retry.spec.tsx— owner rebuild counts for request- and instance-scoped pending sources, body and template reads, plus alazy()route under<Errored>with no<Loading>. The<Document>+<Errored>cases fail onnext(instance-scoped ones time out; the lazy route's layout renders twice).renderToStringfloor 20,377 B / 20.38 KB).Not covered here
renderToStream(...).then) under a shell suspension has its own pre-existing bugs, found while adding.thenvariants: without<Errored>the document resolves empty (seroval'sonDonecompletes the render while a root hole is still pending —doShell()bails and completion runs anyway), and with<Errored>the render hangs synchronously. Both reproduce onnextwithout this change; they'll be a separate PR with the.thenvariants.