Compose: Fix SSR crash in useMediaQuery and useViewportMatch#78725
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @xavier.lozano.carreras@a8c.com. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
mcsf
left a comment
There was a problem hiding this comment.
Looks alright, thanks for the contribution!
SSR testing is sort of novel in Gutenberg, but I don't mind experimenting with some patterns here, especially to unblock SSR in a package like compose.
Follow-up to the previous commit which updated the JSDoc per review feedback on WordPress#78725. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
d96b013 to
7059e80
Compare
Both hooks declared `view = window` as a default parameter. Default expressions are evaluated unconditionally at every call, so the bare `window` identifier threw `ReferenceError: window is not defined` in non-DOM environments (Node SSR, jsdom-less tests) before the function body and the SSR-safe `useSyncExternalStore` server snapshot (`() => false`) had a chance to run. Resolve the default lazily with `typeof window !== 'undefined' ? window : undefined` so the expression itself is safe, and let `getMQLSubscriber` accept `Window | undefined` and short-circuit to `EMPTY_SUBSCRIBER` when `view` is falsy. Adds a `@jest-environment node` regression test that renders both hooks via `react-dom/server`'s `renderToString`. To make that runnable under the shared Jest config, the jsdom-only setup in `jest-preset-default` and `test/unit/config` is guarded with `typeof window !== 'undefined'` (the `@testing-library/user-event` mock stays at the module top level for babel-jest hoisting and falls back to a passthrough when there is no DOM). Regression from WordPress#76446.
Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Follow-up to the previous commit which updated the JSDoc per review feedback on WordPress#78725. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7059e80 to
6b820ed
Compare
What?
Follow up to #76446.
Make
useMediaQueryanduseViewportMatchsafe to call during server-side rendering. Today they throwReferenceError: window is not definedthe moment they're invoked under Node SSR or ajest-environment nodetest, unmounting the whole React tree.Why?
#76446 added an optional
view: Windowargument to both hooks, defaulting to the globalwindow:Default parameter expressions are evaluated unconditionally at every call. In a non-DOM environment,
windowis an unbound identifier, so the bare reference throws aReferenceErrorbefore the function body ever runs — and the existing SSR-safe machinery inside the body (useSyncExternalStore(subscribe, getValue, () => false), with() => falseas the server snapshot) never gets a chance to execute.How?
viewdefault lazily inside the parameter list withtypeof window !== 'undefined' ? window : undefined. The default expression itself is now safe, and consumers passing an explicitvieware unaffected.getMQLSubscriber's signature to acceptWindow | undefinedand short-circuit toEMPTY_SUBSCRIBERwhenviewis falsy, so the type matches the new reality andWeakMap.get(undefined)is impossible.With these changes, SSR renders fall through to the
useSyncExternalStoreserver snapshot and producefalse; the real match is computed on the client after mount, as before.Test coverage
Adds a regression test under
@jest-environment nodethat renders both hooks viareact-dom/server'srenderToString. Without the fix, all three cases throw the originalReferenceError.To make a Node-environment test runnable under the shared Jest config, the jsdom-only setup in
packages/jest-preset-default/scripts/setup-globals.jsandtest/unit/{config,mocks}is guarded withtypeof window !== 'undefined'. The@testing-library/user-eventmock stays at the module top level (needed forbabel-jesthoisting) and falls back to a passthrough when there is no DOM.Use of AI Tools
Authored with assistance from Claude Code (Opus 4.7). All changes were reviewed and tested by the author.