fix(ssr): stop installing a window global during server rendering - #3456
Merged
Conversation
next-themes renders its anti-flash script with
`nonce={typeof window === 'undefined' ? nonce : ''}`. Under veryfront SSR that
ternary took the browser branch, so the script went out as `<script nonce="">`,
and a nonce-based CSP blocked it on every render. Confirmed against production:
the blocked script's hash on codersociety's preview is
sha256-Rcq79NsqIRfp7JA/RMr/1IFEj6q7YAyDVrf6BpzxXwM=, which is exactly the
sha256 of that tag's contents in the served HTML. It was the only element in
the document with an empty nonce; every framework-emitted script carried the
real one.
`typeof window === "undefined"` is how the ecosystem asks whether it is on the
server, and setupSSRGlobals answered "no" by installing a stub. First-party
code was already routed around this -- `isServerEnvironment()` exists for
precisely this reason and consults `__VERYFRONT_SSR__` first -- but a project's
dependencies cannot call it, so the workaround only ever covered half the
problem. Removing the global makes the question answer itself, for everyone.
Nothing the stub exists to provide is withdrawn. `document`, `navigator`,
`location`, `matchMedia`, storage and the DOM constructors are all still
installed, and the constructors remain reachable through `document.defaultView`
for libraries that arrive via an element rather than a bare global -- the
Headless UI focus-manager path the stub was written for. Only the bare `window`
binding goes.
The early-return guard is unchanged and still correct: `window` and `document`
both defined means a real browser, and since this function no longer installs
`window`, that condition can no longer be produced by an earlier call to it.
Verified: a probe evaluating next-themes' exact nonce expression under
setupSSRGlobals emits `""` before this change and the real nonce after. Full
unit suite green -- 3811 passed, 27991 steps, 0 failed. deno check, lint and
fmt clean on src/rendering/ssr-globals.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesSSR global setup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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.
What
setupSSRGlobalsno longer installs a barewindowglobal. Everything else the stub provides —document,navigator,location,matchMedia, storage, DOM constructors — stays, and the constructors remain reachable throughdocument.defaultView.Why
next-themes renders its anti-flash script with:
Under veryfront SSR that ternary took the browser branch, so the script went out as
<script nonce="">, and a nonce-based CSP blocked it on every render.Confirmed against production. The blocked script's hash on codersociety's preview is
sha256-Rcq79NsqIRfp7JA/RMr/1IFEj6q7YAyDVrf6BpzxXwM=, which is exactly the sha256 of that tag's contents in the served HTML. It was the only element in the document with an empty nonce — every framework-emitted script carried the real one.typeof window === "undefined"is how the ecosystem asks whether it is on the server, and the stub answered "no". First-party code was already routed around this —isServerEnvironment()(src/platform/compat/runtime.ts:140) exists for precisely this reason and consults__VERYFRONT_SSR__first — but a project's dependencies cannot call it, so the workaround only ever covered half the problem. Removing the global makes the question answer itself, for everyone.Before / after
A probe evaluating next-themes' exact expression under
setupSSRGlobals:typeof window"object"""→ blocked"undefined"Compatibility
Nothing the stub exists to provide is withdrawn. Libraries needing DOM constructors find them as bare globals; those that arrive via an element (Headless UI's focus manager reads
window.HTMLElement.prototypethroughownerDocument.defaultView) resolve throughdocument.defaultView, wired to the stub.The early-return guard is unchanged and still correct:
windowanddocumentboth defined means a real browser, and since this function no longer installswindow, that condition can no longer be produced by an earlier call to it.Verification
Full unit suite green: 3811 passed, 27995 steps, 0 failed.
deno check,lint,fmtclean onsrc/rendering/ssr-globals.Summary by CodeRabbit
window.