Derive overflow from root-scoped IntersectionObserver entries instead of offsetTop reads#8030
Merged
Conversation
|
| Name | Type |
|---|---|
| @primer/react | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Optimize IntersectionObserver usage in shared registry
Use root-scoped IntersectionObserver entries for overflow detection
Jun 23, 2026
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot
AI
changed the title
Use root-scoped IntersectionObserver entries for overflow detection
Drop per-item overflow observer fallback; use callback-ref shared observer
Jun 23, 2026
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot
AI
changed the title
Drop per-item overflow observer fallback; use callback-ref shared observer
Keep per-item observer fallback removal; revert callback-ref conversion
Jun 23, 2026
13d8541
into
underline-nav-shared-observer-coalesced-rebuild
45 of 50 checks passed
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
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.
Follow-up to the shared-overflow-observer work. The shared
IntersectionObserverin the descendant registry now derives overflow directly from observer entries (isIntersecting/intersectionRatio) and is scoped to the clipping container via a newroot, instead of being used only as a trigger to re-readoffsetTop > 0. This closes the layout-thrash path (no synchronousoffsetTopreads) and removes the per-itemIntersectionObserverfallback.Base branch is
underline-nav-shared-observer-coalesced-rebuild, so this diff is scoped to the changes on top of that work — notmain.Changelog
New
ProviderPropsaccepts arootRefthat is passed to the sharedIntersectionObserveras itsroot, scoping intersection to the actual clipping container (theActionBaroverflow container /UnderlineNavitem list).getIsOverflowing(entry)derives overflow state from the observed entry: any target that isn't fully visible (!isIntersecting || intersectionRatio < 1) counts as overflowing, guarding sub-pixel boundary cases.Changed
isOverflowingper entry and fans that boolean out to subscribers, rather than firing a bareonChange()that forced anoffsetTopre-read.ObserveFn/useRegisterOverflowObservernow propagate the overflow boolean (onOverflowChange(isOverflowing));useSyncExternalStorereads a trackedisOverflowingRefinstead ofref.current.offsetTop > 0.{root, threshold: [0, 1]}; when therootelement changes, the provider re-observes all currently-subscribed elements against the new observer instance (observeSubscribedElements+ an isomorphic layout effect).ActionBarandUnderlineNavpassrootRefto the registryProvider(overflowContainerRef/listRef).overflowoption is simplified from{threshold?: number}to anobjectflag — the explicitthreshold: 1/threshold: 0.75configs onActionBarandUnderlineNavare dropped, since overflow is now read from the entry rather than tuned via threshold.supportsIntersectionObserver().Removed
IntersectionObserverfallback: the hook now tracks overflow only through the provider's shared observer and is inert (false) when no shared observer is configured.offsetToplayout reads from the overflow path (and the related code comments describing theoffsetTop-trigger model).0.75threshold workaround comment inActionBar(no longer needed without threshold tuning).Rollout strategy
Testing & Reviewing
Internal-only refactor; no public API or behavior change. Overflow detection moves from an
offsetTop-trigger model to reading the observedIntersectionObserverEntrydirectly.packages/react/src/utils/__tests__/descendant-registry.test.tsxis updated to match:optionsand exposesemit(entries)so tests can drive real entry shapes (target/isIntersecting/intersectionRatio) instead of a baretrigger().rootelement.offsetTopis stubbed to throw to prove it is never read.unobserveassertion).Perf characteristics worth noting during review:
useSyncExternalStoreare all preserved — siblings don't re-render when one item's overflow flips.offsetTop/layout reads, so the layout-thrash path stays closed; the fallback removal prevents reintroducing anoffsetTop-style local observer.keybump forcing re-registration on membership/order change) is the main remaining cost and is unchanged here.Merge checklist