Compose: Share a single change listener per MediaQueryList in useMediaQuery#78297
Merged
Conversation
…aQuery Every `useMediaQuery` consumer previously created its own `subscribe` closure that attached a `change` listener to the (already-cached) MediaQueryList. With `useViewportMatch` consumed across ~18 block-editor components and ~6 block-library components, a large-post editor mount registers many listeners on the same MediaQueryList. In a representative post-editor first-block CPU profile, this shows up as 494 `addEventListener` samples (~85 ms) on the React commit's critical path, all routed through compose's `subscribe` wrapper. Introduce a `WeakMap<MediaQueryList, MQLSubscriber>` cache that holds a single underlying `change` listener per MediaQueryList and fans out to a `Set` of React callbacks. The hook signature and external behavior are unchanged; the listener-attach/detach moves to first-subscriber / last-unsubscriber boundaries. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
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 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. |
`getSubscriber()` returns a stable per-MediaQueryList subscriber from a `WeakMap`, and `getMediaQueryList()` returns a stable MQL from `perWindowCache`. So the result of resolving them is already referentially stable across renders for the same `(view, query)` and useSyncExternalStore won't re-subscribe. The `useMemo` only added per-render overhead. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The intermediate \`MediaQueryList\` cache wasn't earning its keep — only \`useMediaQuery\` itself consumed it, immediately wrapping the MQL into a subscriber. Other call sites (~8 in the codebase) call \`window.matchMedia\` directly and bypass this file entirely. Collapse into a single \`WeakMap<Window, Map<string, MQLSubscriber>>\`. The MQL still exists, but lives inside the subscriber's closure as a private detail rather than a separately-cached object. One lookup chain, one cache, no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Size Change: +9 B (0%) Total Size: 7.96 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in e4ba176. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25864217254
|
Member
Author
|
There's a clear benefit after two runs. Merging. |
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
1 participant
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?
Share one
changelistener perMediaQueryListacross alluseMediaQueryconsumers, instead of one per consumer.Why?
Each consumer was attaching its own listener via
useSyncExternalStore.useViewportMatchis used in ~24 editor/library components, so a large-post mount registers many listeners on the same MQL — each one a JS↔C++ boundary crossing. Trace shows 494addEventListenersamples (~85 ms) during the React commit undersubscribe @ compose.How?
WeakMap<Window, Map<string, MQLSubscriber>>keyed by(view, query). The subscriber owns one underlying listener that fans out to an in-JSSetof React callbacks; attach on first subscriber, detach on last. Stable identities from the cache let theuseMemogo too.Perf (CI, post-editor)
Testing
npm run test:unit -- packages/compose/src/hooks/use-media-query/Use of AI Tools
Authored with Claude Code assistance; reviewed and verified by the author.