[combobox] Add getValueFromItem prop#4097
Conversation
commit: |
Bundle size report
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. |
d844dfe to
5dd5f3f
Compare
mnajdova
left a comment
There was a problem hiding this comment.
First quick pass, should the name of the prop be itemToValue, to be closer to existing props lie itemToStringValue, itemToStringLabel etc?
|
@mnajdova I explained why it's different:
So:
|
|
Ok, too late to rename those, maybe add it in the v2 ideas. Restructuring the name to have get, doesn't make it more clear that they are receiving different thing as an argument. Ideally those should be named: |
5dd5f3f to
a0096f6
Compare
Codex ReviewOverviewThis patch adds Findings (None)No blocking issues found in this patch. Confidence: 4/5High confidence based on a full branch pass, the expanded regression coverage around mapped values, and a follow-up perf check after the latest simplification. Residual risk is mostly around edge-case tradeoffs in very large lists with custom equality. Notes
|
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
|
Had a long discussion with Codex then fed it into GPT-5.4 Pro to analyze. Here are the conclusions. GPT-5.4 Pro Overview - Combobox keyed values with object itemsSummaryCombobox APIs are easiest to understand when the following all have the same type:
That model works well for:
A common application shape does not fit that model as cleanly:
Example: const users = [
{ id: '1', name: 'Ada' },
{ id: '2', name: 'Grace' },
];
<Combobox.Root items={users} defaultValue="2">
{users.map((user) => (
<Combobox.Item key={user.id} value={user.id}>
{user.name}
</Combobox.Item>
))}
</Combobox.Root>;In this shape, the selected value is not the same thing as the source item. A Combobox that wants to support this cleanly needs an explicit way to derive the selected key from an item. Why a new prop is necessaryReal application data is often normalizedMany applications store data like this: const post = { authorId: '2' };
const authors = [
{ id: '1', name: 'Ada' },
{ id: '2', name: 'Grace' },
];The UI wants a Combobox bound to This is a normal and useful shape because it keeps state small and stable:
Labels and identity are different concernsWhen items are objects, Combobox usually needs at least two distinct pieces of information:
If the selected value is the whole object, those concerns can often be resolved from the same value. That means a stringification prop is not enough on its own. A keyed mode needs:
Existing label/value props are doing different jobsA prop that converts a value to a string is not the same as a prop that derives the selected identity from an item. Those are separate responsibilities:
Trying to overload one existing prop to do all three makes the API harder to understand. Why Base UI needs an explicit keyed modeBase UI is more flexible than many Combobox libraries because it supports both:
That flexibility also makes the relationship between source items and selected values more important to define clearly. In same-shape mode, the rules are simple:
In keyed mode, they do not:
That relationship needs to be stated explicitly in the API rather than emerging indirectly from a combination of label, equality, and serialization props. What other libraries doMost libraries avoid this exact problem by choosing one primary model.
React AriaReact Aria is the clearest mainstream example of first-class keyed selection:
This is the closest precedent for supporting object items with primitive selected values directly. MUI AutocompleteMUI Autocomplete is generally object-first:
When app state stores only an ID, the common pattern is to do an React SelectReact Select also usually keeps the public controlled value as the option object or array of option objects. It exposes helpers like:
But DownshiftDownshift is also usually item-first:
If application state stores only an ID, users typically resolve the selected item outside the component. Headless UI ComboboxHeadless UI typically uses the same value shape as each option:
If objects are used, comparison helpers can change how equality works, but the selected value still remains object-shaped. Why Base UI is differentBase UI sits in a slightly unusual spot because it combines:
That makes a keyed mode both more possible and more worth defining clearly. Other libraries often push the normalization boundary outward: const selectedAuthor = authors.find((author) => author.id === post.authorId) ?? null;
<Autocomplete
options={authors}
value={selectedAuthor}
onChange={(_, author) => setAuthorId(author?.id ?? null)}
/>That approach works, but it means the component API is object-first even when the application state is key-first. A Base UI keyed mode could support the normalized shape directly instead: <Combobox.Root value={post.authorId} items={authors} />That is a meaningful API distinction. Why overloading an existing prop is not enoughIt may be tempting to avoid a new prop by changing how an existing prop is interpreted. For example, one idea is to make an existing stringification prop double as the place where the selected key is inferred. That has a few problems:
If keyed mode is supported, it is better to make that mode explicit. Recommended directionThe smallest clear addition is a dedicated prop for deriving the selected key from an item. For example: <Combobox.Root
items={users}
defaultValue="2"
itemValue="id"
itemToStringLabel={(item) => item.name}
>
{users.map((user) => (
<Combobox.Item key={user.id} value={user.id}>
{user.name}
</Combobox.Item>
))}
</Combobox.Root>Or function form: <Combobox.Root
items={users}
defaultValue="2"
itemValue={(item) => item.id}
itemToStringLabel={(item) => item.name}
>
{users.map((user) => (
<Combobox.Item key={user.id} value={user.id}>
{user.name}
</Combobox.Item>
))}
</Combobox.Root>SemanticsWithout
With
TradeoffsBenefits
Costs
Alternative: object-first onlyA simpler alternative is to keep Combobox fully same-shape and recommend object-first usage: const selectedAuthor = authors.find((author) => author.id === post.authorId) ?? null;
<Combobox.Root
items={authors}
value={selectedAuthor}
itemToStringLabel={(author) => author.name}
itemToStringValue={(author) => author.id}
isItemEqualToValue={(item, selected) => item.id === selected.id}
>
{authors.map((author) => (
<Combobox.Item key={author.id} value={author}>
{author.name}
</Combobox.Item>
))}
</Combobox.Root>This is a valid model, and it matches what many libraries encourage. Its downsides are:
Final recommendationIf Base UI wants to keep the API strictly same-shape, it should document that clearly and treat object-items + primitive-selected-value as out of scope. If Base UI wants to support normalized application data directly, it should add an explicit keyed mode. The cleanest minimal version of that is:
That makes the most common normalized-data use case first-class without requiring a larger API redesign. |
|
Replacing with #4985 |
Fixes #3865
Fixes #3951
Closes #3853
Smaller and more flexible alternative to #3954. This explicitly supports object
itemswith primitivevalue.<Combobox.Root>needs to know what is passed to<Combobox.Item value={...}>—valuecould be a primitivestringornumber, or the whole item object (whenitemsis an array of objects). This new propgetValueFromItemlets it know explicitly.This can't be inferred automatically except in two cases:
{ value, label }is the shape of theitemsobjectsdefaultValue/valuearen'tnullgetValueFromItemsupports arbitrary object shapes and the initialnullvalue.<Combobox.Root>will error ifgetValueFromItemis missing andvalue/defaultValuedon't match the shape ofitems:This does require synchronizing
<Combobox.Item value={...}andgetValueFromItemtogether. I added a wrapper example in the spec file.The
getValueFromItemname is differentiated from the other mappers (itemToStringValue,itemToStringLabel) as those ones operate on item values, not item objects.Explanation why this is needed:
Combobox.Rootowns the selection state and matching logic (selectedIndex, highlight, input label), so it must know the selected value shape before items are interacted with.itemscan be objects, but the selected value can be either:id,value, etc.).Root cannot guess which contract you intend.
Combobox.Item value={...}is not a reliable source for automatic inference at Root level: values can be dynamic, transformed, grouped, virtualized, or rendered in different order.value/defaultValuecan benull, so type/value-shape cannot be inferred from those props at mount time.{ value, label }object shape is the only safe convention Base UI can special-case automatically; arbitrary object shapes are not inferable.getValueFromItemgives Root an explicit mapping from item object -> selected value, so all internal comparisons and label resolution stay consistent.