-
Notifications
You must be signed in to change notification settings - Fork 201
chore: migrate from @radix-ui/* to unified radix-ui package #2033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@strapi/design-system": patch | ||
| "@strapi/ui-primitives": patch | ||
| --- | ||
|
|
||
| chore: migrate from `@radix-ui/*` to the unified `radix-ui` package | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,10 @@ import { | |
| parseAbsolute, | ||
| getLocalTimeZone, | ||
| } from '@internationalized/date'; | ||
| import { useFocusGuards } from '@radix-ui/react-focus-guards'; | ||
| import { FocusScope } from '@radix-ui/react-focus-scope'; | ||
| import { FocusGuards, FocusScope as FocusScopePrimitive } from 'radix-ui/internal'; | ||
|
|
||
| const FocusScope = FocusScopePrimitive.FocusScope; | ||
| const useFocusGuards = FocusGuards.useFocusGuards; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: these |
||
| import { Calendar, Cross } from '@strapi/icons'; | ||
| import { composeEventHandlers } from '@strapi/ui-primitives'; | ||
| import { RemoveScroll } from 'react-remove-scroll'; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,25 @@ | ||
| import * as React from 'react'; | ||
| import { useId } from 'react'; | ||
|
|
||
| import { composeEventHandlers } from '@radix-ui/primitive'; | ||
| import { useComposedRefs } from '@radix-ui/react-compose-refs'; | ||
| import { createContext } from '@radix-ui/react-context'; | ||
| import { DismissableLayer } from '@radix-ui/react-dismissable-layer'; | ||
| import { useFocusGuards } from '@radix-ui/react-focus-guards'; | ||
| import { FocusScope } from '@radix-ui/react-focus-scope'; | ||
| import * as PopperPrimitive from '@radix-ui/react-popper'; | ||
| import { Portal as PortalPrimitive } from '@radix-ui/react-portal'; | ||
| import { Primitive } from '@radix-ui/react-primitive'; | ||
| import { useControllableState } from '@radix-ui/react-use-controllable-state'; | ||
| import { useLayoutEffect } from '@radix-ui/react-use-layout-effect'; | ||
| import { useId, type ComponentPropsWithoutRef } from 'react'; | ||
|
|
||
| import { Portal as RadixPortal } from 'radix-ui'; | ||
| import { | ||
| Context, | ||
| DismissableLayer as DismissableLayerPrimitive, | ||
| FocusGuards, | ||
| FocusScope as FocusScopePrimitive, | ||
| Popper as PopperPrimitive, | ||
| Primitive, | ||
| composeEventHandlers, | ||
| useComposedRefs, | ||
| useControllableState, | ||
| useLayoutEffect, | ||
| } from 'radix-ui/internal'; | ||
|
|
||
| const PortalPrimitive = RadixPortal.Portal; | ||
| const DismissableLayer = DismissableLayerPrimitive.DismissableLayer; | ||
| const FocusScope = FocusScopePrimitive.FocusScope; | ||
| const createContext = Context.createContext; | ||
| const useFocusGuards = FocusGuards.useFocusGuards; | ||
| import { hideOthers } from 'aria-hidden'; | ||
| import * as ReactDOM from 'react-dom'; | ||
| import { RemoveScroll } from 'react-remove-scroll'; | ||
|
|
@@ -22,8 +30,6 @@ | |
|
|
||
| import { VirtualizedViewport } from './VirtualizedViewport'; | ||
|
|
||
| import type { ComponentPropsWithoutRef } from '@radix-ui/react-primitive'; | ||
|
|
||
| const OPEN_KEYS = [' ', 'Enter', 'ArrowUp', 'ArrowDown']; | ||
| const SELECTION_KEYS = ['Enter']; | ||
|
|
||
|
|
@@ -107,6 +113,7 @@ | |
| disabled?: boolean; | ||
| locale?: string; | ||
| onOpenChange?(open: boolean): void; | ||
| // TODO: This might need to be `value: string | undefined` | ||
|
Check warning on line 116 in packages/primitives/src/components/Combobox/Combobox.tsx
|
||
| onValueChange?(value: string): void; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This TODO can be resolved in this PR — the answer is yes, the type should be Evidence that
So the public
Suggest doing it now rather than merging a known-incorrect type with a TODO.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a type "breaking change". I will need first to do some checks in Strapi monorepo to see how is used there. |
||
| onTextValueChange?(textValue: string): void; | ||
| textValue?: string; | ||
|
|
@@ -115,6 +122,7 @@ | |
| value?: string; | ||
| defaultFilterValue?: string; | ||
| filterValue?: string; | ||
| // TODO: This might need to be `value: string | undefined` | ||
|
Check warning on line 125 in packages/primitives/src/components/Combobox/Combobox.tsx
|
||
| onFilterValueChange?(value: string): void; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This TODO can also be resolved now. Unlike That said, the state is declared |
||
| isPrintableCharacter?: (str: string) => boolean; | ||
| visible?: boolean; | ||
|
|
@@ -200,23 +208,27 @@ | |
| */ | ||
| const [open = false, setOpen] = useControllableState({ | ||
| prop: openProp, | ||
| defaultProp: defaultOpen, | ||
| defaultProp: defaultOpen ?? false, | ||
| onChange: onOpenChange, | ||
| caller: COMBOBOX_NAME, | ||
| }); | ||
| const [value, setValue] = useControllableState({ | ||
| const [value, setValue] = useControllableState<string | undefined>({ | ||
| prop: valueProp, | ||
| defaultProp: defaultValue, | ||
| onChange: onValueChange, | ||
| onChange: onValueChange as ((value: string | undefined) => void) | undefined, | ||
| caller: COMBOBOX_NAME, | ||
| }); | ||
| const [textValue, setTextValue] = useControllableState({ | ||
| prop: textValueProp, | ||
| defaultProp: allowCustomValue && !defaultTextValue ? valueProp : defaultTextValue, | ||
| defaultProp: (allowCustomValue && !defaultTextValue ? valueProp : defaultTextValue) as string, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Suggest typing it explicitly like the sibling states instead of casting the value: const [textValue, setTextValue] = useControllableState<string | undefined>({
prop: textValueProp,
defaultProp: allowCustomValue && !defaultTextValue ? valueProp : defaultTextValue,
onChange: onTextValueChange as ((value: string | undefined) => void) | undefined,
caller: COMBOBOX_NAME,
});(And if |
||
| onChange: onTextValueChange, | ||
| caller: COMBOBOX_NAME, | ||
| }); | ||
| const [filterValue, setFilterValue] = useControllableState({ | ||
| const [filterValue, setFilterValue] = useControllableState<string | undefined>({ | ||
| prop: filterValueProp, | ||
| defaultProp: defaultFilterValue, | ||
| onChange: onFilterValueChange, | ||
| onChange: onFilterValueChange as ((value: string | undefined) => void) | undefined, | ||
| caller: COMBOBOX_NAME, | ||
| }); | ||
|
|
||
| const id = useId(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import rewrite is mechanical, but
radix-ui@1.6.0bundles the current primitive versions while the individual packages were pinned to ~2023 versions. It's ~2 years of Radix behavior changes landing at once — focus handling, pointer-events on dismissable layers, positioning — i.e. a real runtime upgrade for every overlay/menu/dialog in consumers, not just a dependency rename.The description should also say this explicitly: Radix primitives are upgraded from the 1.0.x era to the versions bundled in
radix-ui@1.6.0, and the radix install nests React 19 types — consumers on React 18 may want an@types/reactresolution like the one added to this repo's rootpackage.json.