Skip to content

docs: give the real reason a Suspense fallback is read inline at SSR - #1347

Merged
vivek7405 merged 3 commits into
mainfrom
docs/suspense-fallback-timing-reason
Aug 8, 2026
Merged

docs: give the real reason a Suspense fallback is read inline at SSR#1347
vivek7405 merged 3 commits into
mainfrom
docs/suspense-fallback-timing-reason

Conversation

@vivek7405

@vivek7405 vivek7405 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Closes #1324

Summary

Two doc surfaces explained why a <webjs-suspense> .fallback is read inline at SSR by saying a TemplateResult is not serializer-safe. That reason is wrong, and so was the first reason I replaced it with. Checking the source rather than the claim:

  • A TemplateResult is serializer-safe. packages/core/src/html.js:14 returns a plain { _$webjs: 'template', strings, values } object, and it round-trips through stringify / parse intact. So the original justification was not merely secondary, it was false.
  • The data-webjs-prop-* channel is not browser-only. consumePropAttrs at packages/core/src/render-server.js:959 decodes it on the server and assigns onto the instance at :971, before render() is called.

What actually forces the design is narrower than either claim. packages/core/src/webjs-suspense.js:38 defines the element only when customElements exists, so the injectDSD walk skips it at packages/core/src/render-server.js:934 (lookup(tag) finds no class) and no server-side instance ever runs consumePropAttrs. For this one element the only consumer left is connectedCallback, which is far too late for a placeholder whose whole job is to be in the first flushed bytes. Meanwhile renderTemplate renders the fallback to HTML during byte emission and carries it as data-webjs-fallback (render-server.js:403), which processSuspenseElements writes straight into the shell (:1310).

This also removes a contradiction inside the docs site. website/app/docs/ssr/page.ts:161 already says the custom-element .prop round-trip is consumed by the SSR walker before render(), which both the old wording and my first pass talked past.

Changed

  • website/app/docs/suspense/page.ts:94, the docs-site Suspense page.
  • packages/core/AGENTS.md:38, the webjs-suspense.js row of the core module table.
  • packages/core/src/render-server.js:403, the canonical source comment. The issue ruled this out of scope on the reasoning that it "already leads with the full picture". It does not: it is where the serializer claim originated and both doc copies drifted from it, so correcting the copies and leaving the original would guarantee the next reader hits the wrong one. Comment-only, no behaviour change.

Test plan

  • cd website && npm test: 455 server tests pass, 84 browser tests pass, exit 0. That includes website/test/ssr/docs-links.test.ts ("every internal /docs link the docs publish resolves"), which boots a real request handler over the website app and fetches every internal /docs/... link, so it rendered /docs/suspense (200) and resolved the <a href="/docs/loading-states"> (200) inside the edited paragraph.
  • cd website && npx webjs check: all checks pass.
  • The two corrected facts are verified executably, not asserted: stringify on a TemplateResult returns {"_$webjs":"template","strings":[...],"values":[...]} and parse revives it with the same strings / values, and consumePropAttrs / lookup(tag) were read in place.

No new test. The change alters explanatory prose and one comment, and no code path, export, rendered structure, or attribute. The only test that could exist would assert a paragraph against a substring of its own new wording, which pins prose to itself and has no precedent in the repo.

Browser, e2e, smoke, and Bun parity are N/A: no runtime-sensitive surface moves and the one packages/*/src edit is a comment. The two-app dogfood check is N/A for the same reason, and its website half is subsumed by the 455-test SSR suite above, which boots the app through createRequestHandler and renders the edited page.

Note

The issue body prescribed exact replacement wording and ruled render-server.js out of scope. I did not follow either, because the prescribed wording asserts something the source contradicts. Everything turned up here is fixed in this PR; nothing is filed as follow-up work.

Two doc surfaces explained the inline `.fallback` read by saying a
TemplateResult is not serializer-safe. That is true but secondary, and it
points at a fixable-looking obstacle: a reader could conclude that making
TemplateResult serializable would unlock the `data-webjs-prop-*` path, and
spend real time on it.

The load-bearing constraint is timing. `_hydratePropAttrs()` runs from
`connectedCallback`, behind an `if (!isBrowser) return` guard, so the
property channel applies at hydration by construction. A placeholder whose
whole job is to be in the first flushed bytes cannot wait for that, however
serializable it is.

Lead with timing on both surfaces, matching the ordering the source comment
at packages/core/src/render-server.js:403 already uses. The core module
table keeps serializer-safety as the trailing secondary fact, since it is
real and worth knowing when reading `renderTemplate`.
@vivek7405 vivek7405 self-assigned this Aug 8, 2026
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Design rationale: why timing leads and serializer-safety trails

I went back and forth on whether to keep both reasons on both surfaces, and settled on ordering by what a reader can act on.

Serializer-safety reads as a property of the payload, so it invites the reader to change the payload. Timing is a property of the pipeline, and no change to the fallback value moves a hydration-time write earlier than a server flush. That asymmetry is the whole reason the old wording was worth correcting: it was true, but it pointed at a fixable-looking obstacle, and someone could have spent real time trying to make TemplateResult serializable on the theory that it would unlock the property channel.

On the docs page I dropped the serializer clause entirely rather than demoting it. That page is a reader-facing sentence about using the boundary, not about the renderer's internals, and the serializer half is a fact about a channel the reader is being told never to use, so it adds a clause without adding a decision. On packages/core/AGENTS.md I kept it as the trailing fact, because that surface is read alongside renderTemplate and the drop behaviour is genuinely worth knowing there.

I also checked how the neighbours solve this before committing to the shape. React Fizz makes the fallback the boundary segment's rendered children on the server and replaces the segment later (ReactFizzServer.js, "The children of the boundary segment is actually the fallback"), which is structurally what we do with data-webjs-fallback. And lit-ssr's renderPropertyPart returns undefined for any property that is not a reflected IDL attribute, so it emits nothing at all into the HTML for a rich property binding on a custom element. Our data-webjs-prop-* channel is the deliberate divergence from that, and it lands on the client. So there is no server-side prop channel to fall back to in either framework. The corrected sentence is not describing a WebJs quirk, it is describing the only ordering the platform allows.

I left packages/core/src/render-server.js:403 alone on purpose. It already gives both reasons in the right order and is the canonical statement the two doc surfaces had drifted from. Editing it would stage a packages/*/src file and arm the source gates over a change that alters no behaviour.

The first pass replaced one wrong reason with another. Checking the source
rather than the claim:

`html.js:14` returns a plain `{ _$webjs: 'template', strings, values }`
object, and it round-trips through `stringify` / `parse` intact, so "a
TemplateResult is not serializer-safe" was never true. And the
`data-webjs-prop-*` channel is not browser-only either: `consumePropAttrs`
at `render-server.js:959` decodes it server-side and assigns onto the
instance before `render()`.

What actually forces the design is narrower. `webjs-suspense.js:38` defines
the element only when `customElements` exists, so the injectDSD walk skips
it (`lookup(tag)` finds no class) and no server-side instance ever runs
`consumePropAttrs`. For this one element the only remaining consumer is
`connectedCallback`, which is far too late for a placeholder whose whole job
is to be in the first flushed bytes.

Say that on all three surfaces, including the source comment the two doc
copies had drifted from, since it carried the serializer claim that started
this. Also removes the contradiction with `docs/ssr/page.ts:161`, which
correctly says the walker consumes the channel before `render()`.

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through this against the actual source and the first pass was wrong, so I have reworked it.

The premise I inherited from the issue does not survive a read of the code. html.js:14 returns a plain { _$webjs: 'template', strings, values } object and it round-trips through stringify / parse intact, so "a TemplateResult is not serializer-safe" was never true to begin with. That is the claim the issue set out to demote as secondary, and it should have been deleted as false. My replacement was wrong in the other direction: consumePropAttrs at render-server.js:959 decodes data-webjs-prop-* on the server and assigns it before render(), so the channel is not browser-only and "applies at hydration" is false as a general statement about it.

The real constraint is narrower than either. webjs-suspense.js:38 defines the element only when customElements exists, so the injectDSD walk skips it at render-server.js:934 (lookup(tag) finds no class) and no server-side instance runs consumePropAttrs. For this one element the only consumer left is connectedCallback, too late for the first flushed bytes. All three surfaces now say that.

I also pulled render-server.js:403 into scope, which the issue had ruled out. It is where the serializer claim originated and both doc copies drifted from it, so fixing the copies and leaving the original would guarantee the next reader hits the wrong one. Comment-only, no behaviour change.

Comment thread website/app/docs/suspense/page.ts
Comment thread website/app/docs/suspense/page.ts
Comment thread packages/core/AGENTS.md Outdated
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Why the issue's prescribed wording was not followed

The issue was written as a settled plan with exact replacement text, and I departed from it on both surfaces plus one file it ruled out of scope. Recording why, since the diff alone will not show it.

The plan's whole argument was that serializer-safety is a true but secondary fact and timing is the load-bearing one. The first half is wrong. A TemplateResult here is { _$webjs: 'template', strings, values }, a plain object literal, and the wire serializer round-trips it. So the clause the plan wanted demoted to second place on the core module table was not secondary, it was false, and keeping it anywhere would have preserved the error the issue existed to remove.

The second half was directionally right and stated too broadly. "Applies at hydration" is not true of the data-webjs-prop-* channel in general, because the SSR walker consumes it server-side before render(). It is true of <webjs-suspense> specifically, and only because that element is browser-defined, so the walk finds no class for it and skips it. That distinction is the whole content of the correction, and neither the issue nor the wording it prescribed contains it.

Both errors have the same shape as the one being fixed: a claim about the code that reads plausibly and was never run. So I checked these executably before writing them down rather than reasoning from the surrounding prose, which is what produced the original drift.

That is also why render-server.js:403 came into scope after the issue rejected it. The rejection rested on that comment already leading with the full picture. It does not, and it is the origin of the serializer claim both doc copies inherited. Fixing the copies and leaving the source would have left the next reader to find the wrong version first.

@vivek7405
vivek7405 marked this pull request as ready for review August 8, 2026 17:19
Invariant 11 bans a space-surrounded semicolon as pause punctuation. Also unnests the parenthetical, which had a second one inside it.
@vivek7405
vivek7405 merged commit 79fc28f into main Aug 8, 2026
10 checks passed
@vivek7405
vivek7405 deleted the docs/suspense-fallback-timing-reason branch August 8, 2026 19:26
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.

docs: give the real reason a Suspense fallback is read inline at SSR

1 participant