Skip to content

perf(ActionList): memoize context values, menuItemProps, aria attributes #7554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hectahertz wants to merge 6 commits into
base: main
Choose a base branch
from
Open

perf(ActionList): memoize context values, menuItemProps, aria attributes #7554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d3a3e6
perf(ActionList): memoize context values, menuItemProps, aria attributes
hectahertz Feb 15, 2026
38e18ea
changeset
hectahertz Feb 15, 2026
d97ff12
ci: retry flaky test
hectahertz Feb 15, 2026
3eb5a4a
test(ActionList): add parent re-render stress test
hectahertz Feb 15, 2026
c0acd03
revert: remove ParentRerender stress test
hectahertz Feb 15, 2026
79d7a58
Merge branch 'main' into hectahertz/perf-action-list-memoization
hectahertz Feb 18, 2026
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/perf-action-list-memoization.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

perf(ActionList): memoize context values, menuItemProps, and aria attributes
128 changes: 78 additions & 50 deletions packages/react/src/ActionList/Item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ const DivItemContainerNoBox = React.forwardRef<HTMLDivElement, React.HTMLAttribu
},
)

const baseSlots = {
leadingVisual: LeadingVisual,
trailingVisual: TrailingVisual,
trailingAction: TrailingAction,
subItem: SubItem,
}

const slotsConfig = {...baseSlots, description: Description}

// Pre-allocated array for selectableRoles check, avoids per-render allocation
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoist static objects to module scope

baseSlots, slotsConfig, selectableRoles, and listRoleTypes are constant across all renders and all Item instances. Previously they were recreated inside the component body on every render for every item. Moving them to module scope eliminates N object allocations per render (where N = number of items in the list).

const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option']
const listRoleTypes = ['listbox', 'menu', 'list']

const UnwrappedItem = <As extends React.ElementType = 'li'>(
{
variant = 'default',
Expand All @@ -69,14 +82,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
forwardedRef: React.Ref<any>,
): JSX.Element => {
const baseSlots = {
leadingVisual: LeadingVisual,
trailingVisual: TrailingVisual,
trailingAction: TrailingAction,
subItem: SubItem,
}

const [partialSlots, childrenWithoutSlots] = useSlots(props.children, {...baseSlots, description: Description})
const [partialSlots, childrenWithoutSlots] = useSlots(props.children, slotsConfig)
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass stable slot config reference

Previously: useSlots(props.children, {...baseSlots, description: Description}) created a new object via spread on every render. Now it receives the stable module-level slotsConfig.


const slots = {description: undefined, ...partialSlots}

Expand Down Expand Up @@ -150,7 +156,6 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
itemRole === 'menuitemcheckbox' ||
itemRole === 'tab'

const listRoleTypes = ['listbox', 'menu', 'list']
const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics
const buttonSemantics = !listSemantics && !_PrivateItemWrapper

Expand All @@ -165,7 +170,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
const keyPressHandler = React.useCallback(
(event: React.KeyboardEvent<HTMLElement>) => {
if (disabled || inactive || loading) return
if ([' ', 'Enter'].includes(event.key)) {
if (event.key === ' ' || event.key === 'Enter') {
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid array allocation on keypress

Replaced [' ', 'Enter'].includes(event.key) with direct === comparison. The array was allocated on every keypress event handler invocation.

if (event.key === ' ') {
event.preventDefault() // prevent scrolling on Space
// immediately reset defaultPrevented once its job is done
Expand All @@ -189,41 +194,61 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(

const ItemWrapper = _PrivateItemWrapper || DefaultItemWrapper

// only apply aria-selected and aria-checked to selectable items
const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option']
const includeSelectionAttribute = itemSelectionAttribute && itemRole && selectableRoles.includes(itemRole)

let focusable

if (showInactiveIndicator) {
focusable = true
}
const focusable = showInactiveIndicator ? true : undefined
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert mutable let to const ternary

The old let focusable + if block was flagged by the React Compiler as a mutable dependency that couldn't be preserved in memoization. Converting to a const ternary fixes React Compiler compatibility and is more concise.


// Extract the variant prop value from the description slot component
const descriptionVariant = slots.description?.props.variant ?? 'inline'

const menuItemProps = {
onClick: clickHandler,
onKeyPress: !buttonSemantics ? keyPressHandler : undefined,
'aria-disabled': disabled ? true : undefined,
'data-inactive': inactive ? true : undefined,
'data-loading': loading && !inactive ? true : undefined,
tabIndex: focusable ? undefined : 0,
'aria-labelledby': `${labelId} ${slots.trailingVisual ? trailingVisualId : ''} ${
slots.description && descriptionVariant === 'inline' ? inlineDescriptionId : ''
}`,
'aria-describedby':
[
slots.description && descriptionVariant === 'block' ? blockDescriptionId : undefined,
inactiveWarningId ?? undefined,
]
.filter(String)
.join(' ')
.trim() || undefined,
...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}),
role: itemRole,
id: itemId,
}
const hasTrailingVisualSlot = Boolean(slots.trailingVisual)
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memoize aria attribute strings

Previously, aria-labelledby was built with template literals that always produced a new string (including trailing spaces from empty slots). aria-describedby used .filter(String).join(' ').trim() which allocates intermediate arrays on every render.

Now both are computed in useMemo with explicit conditionals. Boolean deps (hasTrailingVisualSlot, hasDescriptionSlot) are derived above to keep the dep array stable and avoid depending on slot object references.

const hasDescriptionSlot = Boolean(slots.description)

const ariaLabelledBy = React.useMemo(() => {
const parts = [labelId]
if (hasTrailingVisualSlot) parts.push(trailingVisualId)
if (hasDescriptionSlot && descriptionVariant === 'inline') parts.push(inlineDescriptionId)
return parts.join(' ')
}, [labelId, hasTrailingVisualSlot, trailingVisualId, hasDescriptionSlot, descriptionVariant, inlineDescriptionId])

const ariaDescribedBy = React.useMemo(() => {
const parts: string[] = []
if (hasDescriptionSlot && descriptionVariant === 'block') parts.push(blockDescriptionId)
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memoize menuItemProps object

This object is created per item and passed to either the <li> (via containerProps) or the <ItemWrapper> (via wrapperProps). Previously rebuilt on every render even when no values changed. Now memoized with all relevant deps.

Note: this has a large dep array (14 deps) because the object touches many item-level values. The memoization still pays off because in typical usage most deps are stable across re-renders (e.g. clickHandler, keyPressHandler, itemRole, itemId rarely change).

if (inactiveWarningId) parts.push(inactiveWarningId)
return parts.length > 0 ? parts.join(' ') : undefined
}, [hasDescriptionSlot, descriptionVariant, blockDescriptionId, inactiveWarningId])

const menuItemProps = React.useMemo(
() => ({
onClick: clickHandler,
onKeyPress: !buttonSemantics ? keyPressHandler : undefined,
'aria-disabled': disabled ? true : undefined,
'data-inactive': inactive ? true : undefined,
'data-loading': loading && !inactive ? true : undefined,
tabIndex: focusable ? undefined : 0,
'aria-labelledby': ariaLabelledBy,
'aria-describedby': ariaDescribedBy,
...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}),
role: itemRole,
id: itemId,
}),
[
clickHandler,
buttonSemantics,
keyPressHandler,
disabled,
inactive,
loading,
focusable,
ariaLabelledBy,
ariaDescribedBy,
includeSelectionAttribute,
itemSelectionAttribute,
selected,
itemRole,
itemId,
],
)

const containerProps = _PrivateItemWrapper
? {role: itemRole ? 'none' : undefined, ...props}
Expand All @@ -238,18 +263,21 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
ref: forwardedRef,
}

Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memoize ItemContext.Provider value

Previously, the inline object passed to ItemContext.Provider was recreated on every render, causing all context consumers (Description, LeadingVisual, TrailingVisual, Selection) to re-render even when their relevant values hadn't changed.

Now wrapped in useMemo with 7 deps. In a 100-item list, this prevents up to ~400 unnecessary child component re-renders per parent state change.

const itemContextValue = React.useMemo(
() => ({
variant,
size,
disabled,
inactive: Boolean(inactiveText),
inlineDescriptionId,
blockDescriptionId,
trailingVisualId,
}),
[variant, size, disabled, inactiveText, inlineDescriptionId, blockDescriptionId, trailingVisualId],
)

return (
<ItemContext.Provider
value={{
variant,
size,
disabled,
inactive: Boolean(inactiveText),
inlineDescriptionId,
blockDescriptionId,
trailingVisualId,
}}
>
<ItemContext.Provider value={itemContextValue}>
<li
{...containerProps}
ref={listSemantics ? forwardedRef : null}
Expand Down
21 changes: 12 additions & 9 deletions packages/react/src/ActionList/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,19 @@ const UnwrappedList = <As extends React.ElementType = 'ul'>(
listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined,
})

const listContextValue = React.useMemo(
Copy link
Contributor Author

@hectahertz hectahertz Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memoize ListContext.Provider value

Same pattern as ItemContext. The inline object was recreated on every ActionList render, causing all ActionList.Item children to re-render due to context referential inequality, even when variant, selectionVariant, showDividers, role, and headingId hadn't changed.

() => ({
variant,
selectionVariant: selectionVariant || containerSelectionVariant,
showDividers,
role: listRole,
headingId,
}),
[variant, selectionVariant, containerSelectionVariant, showDividers, listRole, headingId],
)

return (
<ListContext.Provider
value={{
variant,
selectionVariant: selectionVariant || containerSelectionVariant,
showDividers,
role: listRole,
headingId,
}}
>
<ListContext.Provider value={listContextValue}>
{slots.heading}
{/* @ts-expect-error ref needs a non nullable ref */}
<Component
Expand Down
Loading