Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/defer-anchored-position-mount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Defer `useAnchoredPosition` initial mount setState from useLayoutEffect to useEffect when overlay is closed, eliminating unnecessary cascading re-renders that block paint.
41 changes: 41 additions & 0 deletions packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,47 @@ it('should should return a position', async () => {
})
})

it('should defer initial updatePosition to useEffect when overlay is closed on mount', async () => {
// When no floating element is present (overlay closed), the initial
// updatePosition call should be deferred from useLayoutEffect to useEffect.
// We verify this by checking that onPositionChange has NOT been called by
// the time the component's own useLayoutEffect runs (which fires after the
// hook's useLayoutEffect in declaration order).
const onPositionChange = vi.fn()
const layoutPhaseCheck = vi.fn()

const ClosedOverlayComponent = ({
onPositionChangeProp,
onLayoutEffect,
}: {
onPositionChangeProp: typeof onPositionChange
onLayoutEffect: (calledDuringLayout: boolean) => void
}) => {
const floatingElementRef = React.useRef<HTMLDivElement>(null)
const anchorElementRef = React.useRef<HTMLDivElement>(null)
useAnchoredPosition({floatingElementRef, anchorElementRef, onPositionChange: onPositionChangeProp})

// This layout effect runs after the hook's layout effects (declaration order).
// With the fix, onPositionChange should NOT have been called yet because
// the initial updatePosition is deferred to useEffect.
React.useLayoutEffect(() => {
onLayoutEffect(onPositionChangeProp.mock.calls.length > 0)
}, [onPositionChangeProp, onLayoutEffect])

return <div />
}

render(<ClosedOverlayComponent onPositionChangeProp={onPositionChange} onLayoutEffect={layoutPhaseCheck} />)

// onPositionChange should not have fired during the layout phase
expect(layoutPhaseCheck).toHaveBeenCalledWith(false)

// After effects run, onPositionChange should have been called with undefined
await waitFor(() => {
expect(onPositionChange).toHaveBeenCalledWith(undefined)
})
})

describe('scroll recalculation', () => {
it('should recalculate position when window scrolls', async () => {
const cb = vi.fn()
Expand Down
20 changes: 19 additions & 1 deletion packages/react/src/hooks/useAnchoredPosition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,25 @@ export function useAnchoredPosition(
savedOnPositionChange.current = settings?.onPositionChange
}, [settings?.onPositionChange])

useLayoutEffect(updatePosition, [updatePosition])
// Defer the first updatePosition to useEffect when the overlay is closed on
// mount, avoiding paint-blocking cascading setState. If the overlay is already
// open on mount, run synchronously in useLayoutEffect to prevent a flash.
// After mount (including Suspense reappear), only call updatePosition when
// both refs are attached — skipping closed overlays avoids unnecessary setState.
const hasMountedRef = React.useRef(false)
useLayoutEffect(() => {
if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) {
hasMountedRef.current = true
updatePosition()
}
}, [updatePosition, floatingElementRef, anchorElementRef])

React.useEffect(() => {
if (!hasMountedRef.current) {
hasMountedRef.current = true
updatePosition()
}
}, [updatePosition])
Comment thread
LisaKr marked this conversation as resolved.

useResizeObserver(updatePosition) // watches for changes in window size
useResizeObserver(updatePosition, floatingElementRef as React.RefObject<HTMLElement | null>) // watches for changes in floating element size
Expand Down
Loading