-
Notifications
You must be signed in to change notification settings - Fork 647
perf(hooks): Optimize useAnchoredPosition to avoid duplicate observers and throttle updates #7336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…s and throttle updates - Use window resize listener instead of ResizeObserver on documentElement - Add ResizeObserver for floating element with first-immediate throttling - Use updatePositionRef to avoid callback identity changes - Deduplicate observer setup to avoid redundant work Part of #7312
🦋 Changeset detectedLatest commit: 3578157 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR optimizes the useAnchoredPosition hook to improve Interaction to Next Paint (INP) performance by reducing unnecessary observer overhead and throttling position updates. The changes replace expensive ResizeObserver usage on documentElement with a targeted window resize listener, implement first-immediate throttling for element resize observations, and use a ref pattern to prevent unnecessary effect re-runs.
Key changes:
- Replaced ResizeObserver on documentElement with window resize listener
- Implemented first-immediate throttling pattern for ResizeObserver callbacks using requestAnimationFrame
- Added
updatePositionRefto stabilize callback identity and prevent unnecessary effect re-subscriptions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/hooks/useAnchoredPosition.ts | Removed useResizeObserver hook dependency, added updatePositionRef for callback stabilization, implemented window resize listener and throttled ResizeObserver for floating/anchor elements |
| .changeset/perf-use-anchored-position.md | Added changeset documenting the performance optimization |
Co-authored-by: Copilot <[email protected]>
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/8884 |
🔬 github-ui Integration Test Results
❌ Troubleshooting Failed ChecksProjects (Memex)This check runs Playwright end-to-end tests for the Projects feature. Check the test artifacts for screenshots and traces. Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance. |
Summary
Performance optimizations for useAnchoredPosition hook to improve INP.
Changes
Expected INP Impact
Why this matters
useAnchoredPosition is used by Tooltip, ActionMenu, SelectPanel, Autocomplete, and other overlay components. Problems with the previous implementation:
Window resize + targeted element observation + throttling = much better INP.
Part of the INP performance optimization effort. See #7312 for full context.