refactor(Select): migrate from @radix-ui/react-select to @base-ui/react/combobox#599
refactor(Select): migrate from @radix-ui/react-select to @base-ui/react/combobox#599aammami-ledger wants to merge 33 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Refactors the Select component in libs/ui-react to replace Radix Select with Base UI Combobox, updating the public API and surrounding docs/examples accordingly.
Changes:
- Removed
@radix-ui/react-selectusage and switchedSelectinternals to@base-ui/react/combobox. - Introduced
items-driven rendering, optional search, and filtering/async-filtering hooks (filter,filteredItems,onInputValueChange). - Updated tests, Storybook stories, MDX docs, and Figma Code Connect examples to match the new composition (
SelectList,SelectSearch,SelectEmptyState).
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Removes @radix-ui/react-select dependency from the workspace root. |
| package-lock.json | Removes Radix Select packages; updates workspace metadata (but currently has a peerDependency mismatch noted in comments). |
| libs/ui-react/vite.config.ts | Removes @radix-ui/react-select from Rollup externals. |
| libs/ui-react/src/lib/Components/Select/types.ts | Updates Select public types for Combobox-based API (items, search/filtering props, null selection). |
| libs/ui-react/src/lib/Components/Select/index.ts | Re-exports newly added types. |
| libs/ui-react/src/lib/Components/Select/SelectContext.tsx | Extends context to support search registration and null selected value. |
| libs/ui-react/src/lib/Components/Select/Select.tsx | Re-implements Select using Base UI Combobox; adds list/search/empty-state subcomponents. |
| libs/ui-react/src/lib/Components/Select/Select.test.tsx | Updates and expands tests for the new API and search behavior. |
| libs/ui-react/src/lib/Components/Select/Select.stories.tsx | Updates stories and adds search/custom-filter/async-search examples. |
| libs/ui-react/src/lib/Components/Select/Select.mdx | Updates documentation to the new composition and Base UI foundation. |
| libs/ui-react/src/lib/Components/Select/Select.figma.tsx | Updates Figma Code Connect example to include items + SelectList. |
| libs/ui-react/src/lib/Components/SearchInput/types.ts | Allows containerClassName to be passed through for SelectSearch styling. |
| libs/ui-react/src/lib/Components/BaseInput/BaseInput.tsx | Uses shared useMergedRef utility and slightly reorders props spreading. |
| libs/ui-react/package.json | Removes @radix-ui/react-select from peer dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
19c34fb to
2e2e9a1
Compare
10d2740 to
85e74e6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
libs/ui-react/src/lib/Components/Select/Select.tsx:453
SelectTriggerButtonuses a truthy check (selectedValue ? … : …) to decide whether to show the selected content. SinceselectedValueis typed asstring | null, an empty-string value ('') would be treated as “no selection” even if it’s a valid option value. Prefer an explicit null check (e.g.,selectedValue != null) so empty-string values behave correctly.
const SelectTriggerButton = ({
selectedValue,
selectedContent,
label,
...props
}: SelectTriggerButtonProps) => (
<TriggerButton {...props}>
{selectedValue ? selectedContent : label}
</TriggerButton>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… to @base-ui/react/combobox
| }; | ||
|
|
||
| export type SelectItemGroup = { | ||
| value: string; |
There was a problem hiding this comment.
[mid]: suggestion to use label instead of value here as it represent the name displayed.
Also I would suggest to add JSDoc to clearly understand the API for future updates, even if this one is an internal
gamegee
left a comment
There was a problem hiding this comment.
✅ Ok LGTM
However I see that SonarQube is failing 🤔
If we have time because its lot of code, run the pr-review locally - it might find some additional feedbacks.
This was a big refactor, Good job 👌
There was a problem hiding this comment.
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| consumerName: 'SelectInputTrigger', | ||
| contextRequired: true, | ||
| }); | ||
| const hasValue = selectedValue != null && selectedValue !== ''; |
There was a problem hiding this comment.
The SelectInputTrigger line 130 checks selectedValue != null && selectedValue !== '', but since selectedValue is now string | null (was changed from string), the !== '' check is redundant. After != null passes, selectedValue is guaranteed to be a non-null string. While this doesn't cause bugs, it could be simplified to just selectedValue != null or selectedValue !== null.
| const hasValue = selectedValue != null && selectedValue !== ''; | |
| const hasValue = selectedValue != null; |
| type UseSelectItemsReturn = { | ||
| isGrouped: boolean; | ||
| groupedItems: SelectItemGroup[] | null; | ||
| filteredItemsForRoot: SelectItemData[] | SelectItemGroup[]; |
There was a problem hiding this comment.
[mid]: This pattern is used in multiple places SelectItemData[] | SelectItemGroup[];
Maybe we can expose a type named:
type SelectItemDataUnion = SelectItemData[] | SelectItemGroup[]There was a problem hiding this comment.
actualy only here it's being used
|




Omitted Base UI part
Combobox.PortalSelectContentCombobox.PositionerSelectContentCombobox.PopupSelectContentCombobox.ItemIndicatorSelectItem(always renders a check icon)Combobox.ValueSelectTrigger— not exportedCombobox.CollectionSelectListfor grouped iterationCombobox.LabelCombobox.InputGroupSelectSearchhandles input layoutCombobox.IconCombobox.ArrowCombobox.BackdropCombobox.Chips/Chip/ChipRemoveCombobox.RowCombobox.ClearSearchInput's own clear buttonCombobox.StatususeFilter/useFilteredItemsfilterprop onSelectRoot props mapping
Combobox.RootSelectvalueValue | Value[] | null(generic)string | nulldefaultValueValue | Value[] | nullstringonValueChange(value, eventDetails) => void(value: string | null) => voideventDetailsonInputValueChange(inputValue, eventDetails) => void(value: string) => voideventDetailsonOpenChange(open, eventDetails) => void(open: boolean) => voideventDetailsitemsreadonly Value[]SelectItemData[] | SelectItemGroup[]{ value, label }filteredItemsfilter(item, inputValue) => boolean | null(item, query) => boolean | nullSelectSearchis mountedopen/defaultOpendisabledbooleanbooleanDisabledContextname/requiredmultiplebooleanautoHighlightbooleanhighlightItemOnHoverbooleanautoCompletestringitemToStringLabel(value) => string{ value, label }stringsitemToStringValue(value) => stringisItemEqualToValue(a, b) => booleanactionsRefRefObject<Actions>onItemHighlighted(value, details) => void