Skip to content

Commit 619175c

Browse files
authored
perf: defer useAnchoredPosition setState on initial mount (#7770)
1 parent a217832 commit 619175c

3 files changed

Lines changed: 65 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
Defer `useAnchoredPosition` initial mount setState from useLayoutEffect to useEffect when overlay is closed, eliminating unnecessary cascading re-renders that block paint.

packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,47 @@ it('should should return a position', async () => {
3535
})
3636
})
3737

38+
it('should defer initial updatePosition to useEffect when overlay is closed on mount', async () => {
39+
// When no floating element is present (overlay closed), the initial
40+
// updatePosition call should be deferred from useLayoutEffect to useEffect.
41+
// We verify this by checking that onPositionChange has NOT been called by
42+
// the time the component's own useLayoutEffect runs (which fires after the
43+
// hook's useLayoutEffect in declaration order).
44+
const onPositionChange = vi.fn()
45+
const layoutPhaseCheck = vi.fn()
46+
47+
const ClosedOverlayComponent = ({
48+
onPositionChangeProp,
49+
onLayoutEffect,
50+
}: {
51+
onPositionChangeProp: typeof onPositionChange
52+
onLayoutEffect: (calledDuringLayout: boolean) => void
53+
}) => {
54+
const floatingElementRef = React.useRef<HTMLDivElement>(null)
55+
const anchorElementRef = React.useRef<HTMLDivElement>(null)
56+
useAnchoredPosition({floatingElementRef, anchorElementRef, onPositionChange: onPositionChangeProp})
57+
58+
// This layout effect runs after the hook's layout effects (declaration order).
59+
// With the fix, onPositionChange should NOT have been called yet because
60+
// the initial updatePosition is deferred to useEffect.
61+
React.useLayoutEffect(() => {
62+
onLayoutEffect(onPositionChangeProp.mock.calls.length > 0)
63+
}, [onPositionChangeProp, onLayoutEffect])
64+
65+
return <div />
66+
}
67+
68+
render(<ClosedOverlayComponent onPositionChangeProp={onPositionChange} onLayoutEffect={layoutPhaseCheck} />)
69+
70+
// onPositionChange should not have fired during the layout phase
71+
expect(layoutPhaseCheck).toHaveBeenCalledWith(false)
72+
73+
// After effects run, onPositionChange should have been called with undefined
74+
await waitFor(() => {
75+
expect(onPositionChange).toHaveBeenCalledWith(undefined)
76+
})
77+
})
78+
3879
describe('scroll recalculation', () => {
3980
it('should recalculate position when window scrolls', async () => {
4081
const cb = vi.fn()

packages/react/src/hooks/useAnchoredPosition.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,25 @@ export function useAnchoredPosition(
116116
savedOnPositionChange.current = settings?.onPositionChange
117117
}, [settings?.onPositionChange])
118118

119-
useLayoutEffect(updatePosition, [updatePosition])
119+
// Defer the first updatePosition to useEffect when the overlay is closed on
120+
// mount, avoiding paint-blocking cascading setState. If the overlay is already
121+
// open on mount, run synchronously in useLayoutEffect to prevent a flash.
122+
// After mount (including Suspense reappear), only call updatePosition when
123+
// both refs are attached — skipping closed overlays avoids unnecessary setState.
124+
const hasMountedRef = React.useRef(false)
125+
useLayoutEffect(() => {
126+
if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) {
127+
hasMountedRef.current = true
128+
updatePosition()
129+
}
130+
}, [updatePosition, floatingElementRef, anchorElementRef])
131+
132+
React.useEffect(() => {
133+
if (!hasMountedRef.current) {
134+
hasMountedRef.current = true
135+
updatePosition()
136+
}
137+
}, [updatePosition])
120138

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

0 commit comments

Comments
 (0)