fix(ssr): preload client reference modules before first SSR render#257
fix(ssr): preload client reference modules before first SSR render#257gagipro wants to merge 1 commit intocloudflare:mainfrom
Conversation
commit: |
|
This looks like a reasonable idea, but I don't think this saves other (edge) cases where user's are manually writing |
|
Thanks for the review @hi-ogawa — good points. You're right that this fix only covers the A couple of questions:
This PR targets the common case (every |
|
Just in case, to clarify, I was only commenting as I maintain
The one I mentioned is already fixed facebook/react#33937. Sorry, my comment wasn't really based, but I thought it's worth mentioning.
I'd imagine something like this |
|
Thanks for the clarification @hi-ogawa. After looking at this more carefully, the
So this PR correctly scopes to the The 500-on-first-request is a hard blocker for production deployments, and the virtual module import is already public API (used in the generated SSR entry) — would you be OK merging this as a short-term fix? Happy to contribute a |
On the first request after server start, client reference modules
("use client" components) are loaded lazily via async import(). When
handleSsr calls renderToReadableStream, React SSR tries to render the
HTML shell synchronously but can't resolve client components that
haven't been imported yet. Since there is no <Suspense> boundary
wrapping the root, renderToReadableStream rejects with an undefined
error, causing a 500 on every first request after restart.
Fix: eagerly preload all client reference modules at the start of
handleSsr by calling __vite_rsc_client_require__ for each ID in the
virtual:vite-rsc/client-references map. The existing memoize cache in
setRequireModule ensures this is only async on the very first call;
all subsequent requests resolve from cache immediately.
|
Rebasing this PR against current Re-verified the bug still exists on v0.0.27 (latest release):
Tested 5 restart cycles with concurrent requests ( Force-pushing the rebased branch now. |
b4e0bef to
d70c41f
Compare
|
/bigbonk review |
|
UnknownError: "Overloaded" |
|
@james-elicx Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
|
/bigbonk review |
|
UnknownError: "Overloaded" |
There was a problem hiding this comment.
Thanks for the fix — the bug is real and the root cause analysis is correct. A few things to resolve before merging.
Blocking
No regression test
Per the project guidelines, tests come first. There's no test covering the cold-start behavior. Without one, this can silently regress. Please add a case to the relevant test file (likely tests/app-router.test.ts or tests/features.test.ts) that verifies the first request after server start returns 200.
Silent error swallowing on preload failure
globalThis.__vite_rsc_client_require__(id).catch?.(() => {})If a module fails to load here, the error is discarded, and renderToReadableStream will fail with a confusing/missing error instead of the real cause. At minimum, log in non-production:
globalThis.__vite_rsc_client_require__(id).catch?.((err) => {
if (process.env.NODE_ENV !== "production") {
console.warn("[vinext] failed to preload client ref:", id, err);
}
})Non-blocking
Prod-server parity check
AGENTS.md notes that server/prod-server.ts has its own SSR/routing logic that must be kept in sync with the SSR entry. Worth confirming the same cold-start issue doesn't exist on that path (or noting explicitly that it doesn't apply).
Unnecessary optional chaining
_clientRefs?.default— if this virtual module always exists in the build, the?.on the module itself is dead code..catch?.()— native Promises always have.catch; the optional chaining adds no safety here.
Neither is a bug, just noise.
Summary
Fixes #256 — the first HTTP request after
vinext startalways returns 500.Problem
On cold start, client reference modules (
"use client"components) are loaded lazily viaasync
import()through@vitejs/plugin-rsc'ssetRequireModule({ load }). WhenhandleSsrruns
renderToReadableStreamon the first request, React SSR tries to render the HTML shellbut the client component imports haven't resolved yet. With no
<Suspense>boundary wrappingthe root, React cannot render a fallback and rejects with
undefined, causing a 500.All subsequent requests work because
memoize()insetRequireModulecaches resolved modules.Fix
Eagerly preload all client reference modules at the start of
handleSsr, beforerenderToReadableStreamruns:The
virtual:vite-rsc/client-referencesmodule (provided by@vitejs/plugin-rsc) exposesthe map of all client component IDs. The
__vite_rsc_client_require__global is already setby
initialize()beforehandleSsris called.Scope
packages/vinext/src/server/app-dev-server.ts— insidegenerateSsrEntry()template@vitejs/plugin-rscor any other packageTesting
Verified on a production vinext app with 11 client component references:
Notes
memoize()cache means only the first call per IDdoes actual work. The overhead on subsequent requests is negligible (synchronous Map lookups).
?.defaultand?.catchguards ensure the fix is safe if@vitejs/plugin-rscchanges its export shape in the future.
generateSsrEntry()), so itaffects both dev and production SSR.