Conversation
🦋 Changeset detectedLatest commit: af32e81 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
Pull request overview
Improves @primer/react’s ActionMenu render performance by attempting to stabilize context provider values so context consumers don’t re-render due solely to new object/function identities.
Changes:
- Memoized
MenuContext.Providervalue inMenu. - Memoized
ActionListContainerContext.Providervalue inOverlay. - Extracted
afterSelectinto a stableuseCallback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/src/ActionMenu/ActionMenu.tsx | Memoizes context values and stabilizes afterSelect to reduce unnecessary re-renders. |
| .changeset/perf-action-menu-memo-context.md | Adds a patch changeset entry describing the perf improvement. |
| const menuContextValue = useMemo( | ||
| () => ({ | ||
| anchorRef, | ||
| renderAnchor, | ||
| anchorId, | ||
| open: combinedOpenState, | ||
| onOpen, | ||
| onClose, | ||
| isSubmenu, | ||
| }), | ||
| [anchorRef, renderAnchor, anchorId, combinedOpenState, onOpen, onClose, isSubmenu], |
There was a problem hiding this comment.
menuContextValue will still change on every render because renderAnchor is reassigned to a newly-created inline function during the React.Children.map(...) pass. Since renderAnchor is in the useMemo deps, this memoization won’t prevent re-renders of MenuContext consumers in the common case. Consider memoizing renderAnchor (and/or the contents computation that sets it) so the function identity is stable when children/anchorRef haven’t changed, otherwise the useMemo here provides little/no benefit.
Closes #
Both
MenuContext.ProviderandActionListContainerContext.Providerin ActionMenu were creating new object references on every render. This caused all context consumers (menu items, anchors) to re-render even when no values actually changed.Wrapped both context values in
useMemoand extracted the inlineafterSelectcallback intouseCallback.Changelog
New
N/A
Changed
MenuContextvalue inMenucomponentActionListContainerContextvalue inOverlaycomponentafterSelectinto a stableuseCallbackRemoved
N/A
Rollout strategy
Testing & Reviewing
All 39 existing ActionMenu tests pass. This is a pure memoization change with no behavioral difference.
Merge checklist