Skip to content

perf(hooks): Optimize useAnchoredPosition to avoid duplicate observers and throttle updates - #7336

Closed
mattcosta7 wants to merge 4 commits into
mainfrom
perf/use-anchored-position
Closed

perf(hooks): Optimize useAnchoredPosition to avoid duplicate observers and throttle updates#7336
mattcosta7 wants to merge 4 commits into
mainfrom
perf/use-anchored-position

Conversation

@mattcosta7

@mattcosta7mattcosta7 commented Dec 15, 2025

Copy link
Copy Markdown
Contributor

Summary

Performance optimizations for useAnchoredPosition hook to improve INP.

Changes

  1. Window resize listener - Use window resize instead of ResizeObserver on documentElement
  2. Throttled ResizeObserver - Add ResizeObserver for floating element with first-immediate throttling pattern
  3. updatePositionRef - Avoid callback identity changes that cause unnecessary effect re-runs
  4. Deduplicate observers - Avoid redundant observer setup

Expected INP Impact

ScenarioBeforeAfterImprovement
Worst case (multiple overlays, rapid resize)~80-150ms cumulative<20ms75-87% reduction
Average case (single overlay, occasional resize)~30-50ms per resize<10ms67-80% reduction
Best case (static positioning)~10-20ms initial<5ms50-75% reduction

Why this matters

useAnchoredPosition is used by Tooltip, ActionMenu, SelectPanel, Autocomplete, and other overlay components. Problems with the previous implementation:

  1. ResizeObserver on documentElement - Fired on ANY document size change, extremely expensive
  2. No throttling - Multiple position updates per frame during animations
  3. Callback identity changes - Caused unnecessary effect re-runs

Window resize + targeted element observation + throttling = much better INP.


Part of the INP performance optimization effort. See #7312 for full context.

…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-bot

changeset-botBot commented Dec 15, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 747e815

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Dec 15, 2025
@github-actions

Copy link
Copy Markdown
Contributor

👋 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 integration-tests: skipped manually label to skip these checks.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updatePositionRef to 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.

FileDescription
packages/react/src/hooks/useAnchoredPosition.tsRemoved useResizeObserver hook dependency, added updatePositionRef for callback stabilization, implemented window resize listener and throttled ResizeObserver for floating/anchor elements
.changeset/perf-use-anchored-position.mdAdded changeset documenting the performance optimization

Comment threadpackages/react/src/hooks/useAnchoredPosition.ts
Comment threadpackages/react/src/hooks/useAnchoredPosition.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mattcosta7mattcosta7 self-assigned this Dec 15, 2025
@mattcosta7
mattcosta7 marked this pull request as ready for review December 15, 2025 20:55
@mattcosta7
mattcosta7 requested a review from a team as a code ownerDecember 15, 2025 20:55
@github-actions
github-actionsBot requested a deployment to storybook-preview-7336 December 15, 2025 20:58 Abandoned
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7336 December 15, 2025 21:07 Inactive
@primer-integration

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/8884

@mattcosta7
mattcosta7 marked this pull request as draft December 15, 2025 21:35
@primer-integration

Copy link
Copy Markdown

🔬 github-ui Integration Test Results

CheckStatusDetails
CI✅ PassedView run
Projects (Memex)❌ FailedView run
VRT✅ PassedView run

❌ Troubleshooting Failed Checks

Projects (Memex)

This check runs Playwright end-to-end tests for the Projects feature. Check the test artifacts for screenshots and traces.

👉 View detailed logs


Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance.

@mattcosta7

Copy link
Copy Markdown
ContributorAuthor

superceeded by #7362

@primer
primerBot deleted the perf/use-anchored-position branch June 30, 2026 18:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mattcosta7