Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
21 changes: 20 additions & 1 deletion packages/react/src/hooks/useAnchoredPosition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,26 @@ 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, all updates use useLayoutEffect.
const hasMountedRef = React.useRef(false)
useLayoutEffect(() => {
if (hasMountedRef.current) {
updatePosition()
} else 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