Skip to content

Commit adc970a

Browse files
committed
changeset
1 parent a1a2a67 commit adc970a

2 files changed

Lines changed: 89 additions & 46 deletions

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+
perf(Autocomplete): eliminate re-renders on arrow-key navigation by applying highlight via DOM

packages/react/src/Autocomplete/AutocompleteMenu.tsx

Lines changed: 84 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,57 @@ function getdefaultCheckedSelectionChange<T extends AutocompleteMenuItem>(
4747

4848
const isItemSelected = (itemId: string, selectedItemIds: Array<string>) => selectedItemIds.includes(itemId)
4949

50+
/**
51+
* Memoized wrapper around ActionList.Item that skips re-rendering when props
52+
* haven't changed. Arrow-key navigation no longer triggers re-renders at all
53+
* because the active highlight is applied directly to the DOM via the
54+
* `data-active` attribute in the `onActiveDescendantChanged` callback.
55+
*
56+
* This works because `selectableItems` useMemo no longer depends on
57+
* `highlightedItem`, so the item object references stay stable across
58+
* highlight changes.
59+
*/
60+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
61+
const MemoizedAutocompleteItem = React.memo(function MemoizedAutocompleteItem<T extends Record<string, any>>({
62+
item,
63+
}: {
64+
item: T
65+
}) {
66+
const {
67+
id,
68+
onAction,
69+
children,
70+
text,
71+
leadingVisual: LeadingVisual,
72+
trailingVisual: TrailingVisual,
73+
key,
74+
role,
75+
...itemProps
76+
} = item
77+
return (
78+
<ActionList.Item
79+
key={(key ?? id) as string | number}
80+
onSelect={() => onAction(item)}
81+
{...itemProps}
82+
id={id}
83+
data-id={id}
84+
role={role as AriaRole}
85+
>
86+
{LeadingVisual && (
87+
<ActionList.LeadingVisual>
88+
{isElement(LeadingVisual) ? LeadingVisual : <LeadingVisual />}
89+
</ActionList.LeadingVisual>
90+
)}
91+
{(children ?? text) as React.ReactNode}
92+
{TrailingVisual && (
93+
<ActionList.TrailingVisual>
94+
{isElement(TrailingVisual) ? TrailingVisual : <TrailingVisual />}
95+
</ActionList.TrailingVisual>
96+
)}
97+
</ActionList.Item>
98+
)
99+
})
100+
50101
function getItemById<T extends AutocompleteMenuItem>(itemId: string, items: T[]) {
51102
return items.find(item => item.id === itemId)
52103
}
@@ -167,10 +218,12 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
167218
} = props
168219
const listContainerRef = useRef<HTMLDivElement>(null)
169220
const allItemsToRenderRef = useRef<T[]>([])
170-
const [highlightedItem, setHighlightedItem] = useState<T>()
221+
// Using a ref instead of state to avoid triggering a re-render of all items
222+
// on every arrow-key press. The active styling is applied directly to the DOM
223+
// via the `data-active` attribute (which ActionList.Item already styles).
224+
const highlightedItemRef = useRef<T>()
171225
const [sortedItemIds, setSortedItemIds] = useState<Array<string>>(items.map(({id: itemId}) => itemId))
172226
const generatedUniqueId = useId(id)
173-
const highlightedItemId = highlightedItem?.id
174227

175228
const selectableItems = useMemo(
176229
() =>
@@ -289,15 +342,32 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
289342
return !(element instanceof HTMLInputElement)
290343
},
291344
activeDescendantFocus: inputRef,
292-
onActiveDescendantChanged: (current, _previous, directlyActivated) => {
345+
onActiveDescendantChanged: (current, previous, directlyActivated) => {
293346
activeDescendantRef.current = current || null
347+
348+
// Toggle the active styling directly on the DOM elements to avoid
349+
// re-rendering every item in the list. ActionList.Item already styles
350+
// `[data-active]` so we just set/remove the attribute.
351+
const previousLi = previous?.closest('li')
352+
const currentLi = current?.closest('li')
353+
if (previousLi) previousLi.removeAttribute('data-active')
354+
if (currentLi) currentLi.setAttribute('data-active', 'true')
355+
294356
if (current) {
295357
const selectedItem = allItemsToRenderRef.current.find(item => {
296-
return item.id === current.closest('li')?.getAttribute('data-id')
358+
return item.id === currentLi?.getAttribute('data-id')
297359
})
298360

299-
setHighlightedItem(selectedItem)
361+
highlightedItemRef.current = selectedItem
300362
setIsMenuDirectlyActivated(directlyActivated)
363+
364+
// Update autocomplete suggestion inline (moved from the useEffect
365+
// that previously depended on highlightedItem state)
366+
if (selectedItem?.text?.startsWith(deferredInputValue) && !selectedItemIds.includes(selectedItem.id)) {
367+
setAutocompleteSuggestion(selectedItem.text)
368+
} else {
369+
setAutocompleteSuggestion('')
370+
}
301371
}
302372

303373
if (current && customScrollContainerRef && customScrollContainerRef.current && directlyActivated) {
@@ -311,14 +381,15 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
311381
)
312382

313383
useEffect(() => {
314-
// Use deferredInputValue to avoid running this effect on every keystroke
315-
// The Input component guards against stale suggestions
316-
if (highlightedItem?.text?.startsWith(deferredInputValue) && !selectedItemIds.includes(highlightedItem.id)) {
317-
setAutocompleteSuggestion(highlightedItem.text)
384+
// When the input value changes, update the autocomplete suggestion based
385+
// on the currently highlighted item (tracked via ref, not state).
386+
const highlighted = highlightedItemRef.current
387+
if (highlighted?.text?.startsWith(deferredInputValue) && !selectedItemIds.includes(highlighted.id)) {
388+
setAutocompleteSuggestion(highlighted.text)
318389
} else {
319390
setAutocompleteSuggestion('')
320391
}
321-
}, [highlightedItem, deferredInputValue, selectedItemIds, setAutocompleteSuggestion])
392+
}, [deferredInputValue, selectedItemIds, setAutocompleteSuggestion])
322393

323394
useEffect(() => {
324395
const itemIdSortResult = [...sortedItemIds].sort(
@@ -361,42 +432,9 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
361432
id={`${id}-listbox`}
362433
aria-labelledby={ariaLabelledBy}
363434
>
364-
{allItemsToRender.map(item => {
365-
const {
366-
id,
367-
onAction,
368-
children,
369-
text,
370-
leadingVisual: LeadingVisual,
371-
trailingVisual: TrailingVisual,
372-
key,
373-
role,
374-
...itemProps
375-
} = item
376-
return (
377-
<ActionList.Item
378-
key={(key ?? id) as string | number}
379-
onSelect={() => onAction(item)}
380-
{...itemProps}
381-
active={highlightedItemId === id}
382-
id={id}
383-
data-id={id}
384-
role={role as AriaRole}
385-
>
386-
{LeadingVisual && (
387-
<ActionList.LeadingVisual>
388-
{isElement(LeadingVisual) ? LeadingVisual : <LeadingVisual />}
389-
</ActionList.LeadingVisual>
390-
)}
391-
{(children ?? text) as React.ReactNode}
392-
{TrailingVisual && (
393-
<ActionList.TrailingVisual>
394-
{isElement(TrailingVisual) ? TrailingVisual : <TrailingVisual />}
395-
</ActionList.TrailingVisual>
396-
)}
397-
</ActionList.Item>
398-
)
399-
})}
435+
{allItemsToRender.map(item => (
436+
<MemoizedAutocompleteItem key={(item.key ?? item.id) as string | number} item={item} />
437+
))}
400438
</ActionList>
401439
) : emptyStateText !== false && emptyStateText !== null ? (
402440
<div className={classes.EmptyStateWrapper}>{emptyStateText}</div>

0 commit comments

Comments
 (0)