[combobox] Avoid re-rendering every item on each keystroke#4964
[combobox] Avoid re-rendering every item on each keystroke#4964flaviendelangle wants to merge 1 commit into
Conversation
commit: |
Every `<Combobox.Item>` subscribed to `ComboboxDerivedItemsContext`, whose value identity changes on every keystroke (it carries `query` and the freshly filtered item arrays). A context value-identity change re-renders all consumers regardless of `React.memo`, so all mounted items re-rendered on each input change even when their own state and props were unchanged. Items only needed `hasItems` (a stable boolean) and, for the virtualized fallback without an explicit `index`, `flatFilteredItems`. This: - Moves `hasItems` into its own `ComboboxHasItemsContext`, which only flips when `items` toggles between provided/undefined (stable across keystrokes). - Resolves the virtualized fallback index in a dedicated `ComboboxItemVirtualizedIndex` subscriber so only virtualized items (which re-render every keystroke anyway) read the per-keystroke context; non-virtualized items no longer subscribe to it. With this, `React.memo` on `<Combobox.Item>` can bail for items with stable props, so typing no longer re-renders the whole list. Adds `test/performance/tests/combobox.bench.tsx` measuring typing in a 500-item open combobox. Against this benchmark, total render work per typing interaction drops ~3.4x (e.g. 46ms -> 14ms) and per-keystroke item re-renders drop from ~all-items to zero (or to only the items whose membership actually changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6d7d394 to
395e77f
Compare
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle size
PerformanceTotal duration: 1,308.62 ms -37.88 ms(-2.8%) | Renders: 78 (🔺+28) | Paint: 2,035.76 ms +4.35 ms(+0.2%)
9 tests within noise — details Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review (GPT-5.5)Approve after nits are addressed 🟡, because the performance split looks correct and focused validation passed, but the preserved virtualized no- 1. Bugs / Issues1. 🟡 Add coverage for the virtualized no-index path (non-blocking)The PR keeps a special subscriber for virtualized items that do not pass an explicit
if (virtualized && componentProps.index == null) {
return (
<ComboboxItemVirtualizedIndex componentProps={componentProps} forwardedRef={forwardedRef} />
);
}That path is now the only place where those items still read Fix: Add a focused regression test with Benchmark Summary
Correctness Trade-offsThe runtime shape is targeted: Test Coverage AssessmentI ran |
There was a problem hiding this comment.
I've read the PR & code fairly quickly so I might not understand all the subtleties, but why is the combobox using so many contexts (including the new one introduced in this PR)? The Store was introduced to avoid re-renders due to context updates, is there a limitation that prevents it from being used?
There was a problem hiding this comment.
It's largely because of timing issues. I know for certain that ComboboxInputValueContext is necessary to live outside the store, while the others I believe were following your (initial) Select store architecture.
Claude Fable explanation
The extra contexts exist because of a timing limitation: the store is synced from the root's render state in a layout effect i.e. one commit after the render that computed the value. That's fine for most state, but some values must reach consumers in the same render they're computed:
- ComboboxInputValueContext — the controlled input value can't lag a commit behind the DOM or typing breaks (cursor jumps to the end with async suggestions, #2703)
- ComboboxDerivedItemsContext —
filteredItems/flatFilteredItemsare derived from inputValue during render via useMemo. Routing them through the store would show stale filter results for a frame on every keystroke (and writing to the store during render isn't allowed). - ComboboxHasItemsContext (this PR) — same reason: the item registration effects branch on
hasItems, and with async-loaded items a store read would be stale for one commit, so items could register into the wrong registry. It's also intentionally a separate context from the derived-items one precisely because it's stable per keystroke — that separation is what lets React.memo bail. - ComboboxRootContext/ComboboxFloatingContext carry stable object identities (the store itself, floating-ui's context), so they never trigger re-renders.
So context count isn't the re-render hazard — only a context whose value identity changes is, and after this PR the only hot one (derived items) is paid exclusively by the few consumers that genuinely need per-keystroke data.
Problem
Every
<Combobox.Item>subscribes toComboboxDerivedItemsContext, whose value identity changes on every keystroke (it carriesqueryand the freshly-filtered item arrays). A context value-identity change re-renders all consumers regardless ofReact.memo, so every mounted item re-renders on every keystroke, even when its own state and props are unchanged.Items only actually need two things from that context:
hasItems(a stable boolean) and — only for the virtualized fallback without an explicitindex—flatFilteredItems.Fix
hasItemsinto its ownComboboxHasItemsContext. It only flips whenitemstoggles between provided/undefined, so it's stable across keystrokes. (Using a render-time context rather than the store preserves the async-item-load timing — the store updates one render later in a layout effect.)ComboboxItemVirtualizedIndexsubscriber, so only virtualized items (which re-render every keystroke anyway) read the per-keystroke context. Non-virtualized items no longer subscribe to it.With this,
React.memoon<Combobox.Item>can bail for items with stable props, so typing no longer re-renders the whole list. Arrow-key navigation was already efficient (only the highlighted/unhighlighted items re-render) and is unchanged.Benchmark
Adds
test/performance/tests/combobox.bench.tsx: a 500-item open combobox using the documented function-children pattern, with the user typing into the input. Run in the existing harness (production build, headless Chromium, React Profiler, 20 iterations).Total React render work per typing interaction:
"Row "— all 500 stay mounted"Row 25"— narrows to ~11Item re-renders during the same interaction (instrumented separately):
"Row "(membership unchanged)"Row 25"(489 items unmount)The residual 122 are the necessary re-renders — when items unmount, the surviving items' composite indices shift. The fix removes only the wasteful re-renders.
Limitation
The improvement requires item props to be referentially stable so
React.memocan bail.<Combobox.Item value={item}>{item.label}</Combobox.Item>qualifies. Wrapping item content in a freshly-created element each render (e.g.<span>{item.label}</span>) defeatsReact.memoon its own and sees no improvement; memoize such content if needed.Test plan
pnpm test:jsdom Combobox/Autocomplete— 758 pass, 19 skipped, no regressions.indexkeyboard selection still resolves the correct item after filtering (this PR keeps that path fresh via the dedicated subscriber rather than a stale ref).pnpm typescript,pnpm eslint,pnpm prettierall clean.🤖 Generated with Claude Code