Repository navigation
[combobox] Avoid re-rendering every item on each keystroke - #4964
Conversation
commit: |
flaviendelangle
added
type: enhancement
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,284.84 ms +177.08 ms(+16.0%) | Renders: 78 (🔺+28)
10 tests within noise — details Metric alarms
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. |
flaviendelangle
removed
the
component: autocomplete
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.
Review summarySolid, well-targeted fix. The refactor relocates the item's index-resolution logic verbatim and removes the one per-keystroke context subscription that was defeating Verification (run locally on the PR branch)
Correctness analysis
FindingsImportant1. The virtualized-without- 2. Toggling Suggestions (non-blocking)
Strengths
Recommended actions
None of these block merge given the logic-equivalence and green test/lint/format runs. 🤖 Review generated with Claude Code |
Address the GPT-5.5 and Claude review on mui#4964: - Add a regression test for virtualized items rendered without an explicit `index`, the only path that still resolves its index from the filtered set via `ComboboxItemVirtualizedIndex`. After narrowing the list, keyboard highlight/Enter and pointer selection must target the correct filtered item. - Pass `virtualized` from the wrapper down to `ComboboxItemImpl` as a prop instead of re-subscribing to the store in both components. - Document that `virtualized` (and an item's explicit-`index` presence) must be stable per item lifetime, since the wrapper swaps component types on it. - Tighten the `ComboboxItemVirtualizedIndex` JSDoc to attribute the per-keystroke re-render to the parent virtualizer re-windowing the list.
atomiks
left a comment
atomiks
left a comment
There was a problem hiding this comment.
PR review
No blocking or actionable correctness findings. I verified the render-count regression test now guards the core optimization: it passes on the PR and fails on master with 12 item render calls after one unchanged-membership keystroke. The benchmark also measures the intended cost: with the built package, the PR reduces the 500-item Combobox typing render work from ~22-28ms to ~8ms for unchanged membership, and from ~25-31ms to ~9ms when narrowing.
Bugs (1)
1. ℹ️ Flipping virtualized or an item's index at runtime now remounts the item
Location: packages/react/src/combobox/item/ComboboxItem.tsx:281
if (virtualized && componentProps.index == null) {
return (
<ComboboxItemVirtualizedIndex componentProps={componentProps} forwardedRef={forwardedRef} />
);
}Because the wrapper now selects a different component type based on virtualized and whether index is provided, toggling either at runtime remounts the item. That resets refs/effects where master would have re-rendered in place. The PR already documents this, and both inputs are expected to be stable in normal usage, so this is a heads-up rather than a defect.
Failure scenario: A consumer conditionally starts providing index after mount and sees transient item state reset.
Fix: None needed; the trade-off is documented and acceptable for this optimization.
Tests (0)
No findings.
Simplifications (0)
No findings.
Docs (0)
No findings.
Verdict
Approve — correctness is preserved, the render-count regression test now pins the performance fix, and the benchmark independently confirms the intended ~3x render-work reduction.
🤖 Review generated with Codex and Claude Code combined
|
For follow-up: the win here relies on
const ProduceItem = React.memo(function ProduceItem({ item }: { item: Produce }) {
return (
<Combobox.Item value={item}>
<Combobox.ItemIndicator />
<span>{item.label}</span>
</Combobox.Item>
);
});
<Combobox.Collection>
{(item) => <ProduceItem key={item.id} item={item} />}
</Combobox.Collection> |
…roke Follow-up to mui#4964. That PR stops `<Combobox.Item>` subscribing to the per-keystroke derived-items context, so `React.memo` can bail when an item's props are referentially stable. That holds for flat lists with simple string children, but not for the common grouped + rich-children case: `<Combobox.Collection>` re-runs the user's render function for every item on each keystroke, rebuilding a fresh `children` array (e.g. `<ItemIndicator /> + <span>`), so the item memo can never bail. This internalizes the win at the collection layer so users don't have to hand-memoize their item component: - `ComboboxCollection` renders each item through a memoized internal renderer. Because the collection re-renders from its own context subscription (not its parent), the user's render function stays a stable reference, so a stable item + index lets the wrapper bail and the user's render fn never runs. - `AriaCombobox` preserves grouped filtered-item identities when a group's membership is unchanged (`areSameItems`), so the same `group` reference and its `items` flow down and the wrapper can bail. Includes reference/length fast-paths so the common cases are O(1). Adds typing + open benchmarks (flat/simple and grouped/rich, 500 items) and collection tests asserting the user render fn is not re-invoked on a membership-preserving keystroke, plus removal/reorder correctness for the index key strategy.
…roke Follow-up to mui#4964. That PR stops `<Combobox.Item>` subscribing to the per-keystroke derived-items context, so `React.memo` can bail when an item's props are referentially stable. That holds for flat lists with simple string children, but not for the common rich-children case (flat or grouped): `<Combobox.Collection>` re-runs the user's render function for every item on each keystroke, rebuilding a fresh `children` array (e.g. `<ItemIndicator /> + <span>`), so the item memo can never bail. This internalizes the win at the collection layer so users don't have to hand-memoize their item component: - `ComboboxCollection` caches each item's rendered element by item identity and returns it directly. Because the collection re-renders from its own context subscription (not its parent), the user's render function stays a stable reference, so an unchanged item reuses its cached element and React bails out of re-rendering that subtree without re-invoking the render function. Returning the element directly (not wrapped) keeps the user's own `key` as the reconciliation key, so filtering preserves surviving item subtrees instead of remounting whichever item shifts into a given position. - `AriaCombobox` preserves grouped filtered-item identities when a group's membership is unchanged (`areSameItems`), so the same `group` reference and its `items` flow down and the group's cache entry is reused. Reference/length fast-paths keep the common cases O(1). Adds typing + open benchmarks (flat/simple and grouped/rich, 500 items) and collection tests: the render fn is not re-invoked on a membership-preserving keystroke (flat + grouped), removal/reorder correctness, and a surviving item subtree is not remounted when an earlier item is filtered out.
4 participants
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