Skip to content

Commit

Permalink
Fix composeRefs in React 19
Browse files Browse the repository at this point in the history
Callback refs can now return cleanup functions. A composed ref also
needs to invoke those functions upon cleanup.

I also updated the main workspace React version to 19 and fixed a
handful of type-checking issues. However, all packages should still be
compatible with earlier React versions.

Storybook also needed an update, as the previous version was not
compatible with React 19.
  • Loading branch information
dmitri-gb committed Dec 12, 2024
1 parent 4584a37 commit 1b85e04
Show file tree
Hide file tree
Showing 29 changed files with 2,069 additions and 6,201 deletions.
3 changes: 3 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
.next
node_modules
.yarn
dist
storybook-static
8 changes: 5 additions & 3 deletions .storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@ import path from 'path';

const config: StorybookConfig = {
stories: ['../packages/core/**/*.stories.tsx', '../packages/react/**/*.stories.tsx'],

addons: [
getAbsolutePath('@storybook/addon-essentials'),
getAbsolutePath('@storybook/addon-storysource'),
getAbsolutePath('@storybook/addon-webpack5-compiler-swc'),
],

framework: {
name: getAbsolutePath('@storybook/react-webpack5'),
options: {
builder: {
useSWC: true,
},
builder: {},
// enable React strict mode
strictMode: true,
},
},

swc: () => ({
jsc: {
transform: {
Expand Down
49 changes: 49 additions & 0 deletions .yarn/versions/a4655765.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
releases:
"@radix-ui/primitive": patch
"@radix-ui/react-accessible-icon": patch
"@radix-ui/react-accordion": patch
"@radix-ui/react-alert-dialog": patch
"@radix-ui/react-announce": patch
"@radix-ui/react-arrow": patch
"@radix-ui/react-aspect-ratio": patch
"@radix-ui/react-avatar": patch
"@radix-ui/react-checkbox": patch
"@radix-ui/react-collapsible": patch
"@radix-ui/react-collection": patch
"@radix-ui/react-compose-refs": patch
"@radix-ui/react-context-menu": patch
"@radix-ui/react-dialog": patch
"@radix-ui/react-dismissable-layer": patch
"@radix-ui/react-dropdown-menu": patch
"@radix-ui/react-focus-scope": patch
"@radix-ui/react-form": patch
"@radix-ui/react-hover-card": patch
"@radix-ui/react-label": patch
"@radix-ui/react-menu": patch
"@radix-ui/react-menubar": patch
"@radix-ui/react-navigation-menu": patch
"@radix-ui/react-popover": patch
"@radix-ui/react-popper": patch
"@radix-ui/react-portal": patch
"@radix-ui/react-presence": patch
"@radix-ui/react-primitive": patch
"@radix-ui/react-progress": patch
"@radix-ui/react-radio-group": patch
"@radix-ui/react-roving-focus": patch
"@radix-ui/react-scroll-area": patch
"@radix-ui/react-select": patch
"@radix-ui/react-separator": patch
"@radix-ui/react-slider": patch
"@radix-ui/react-slot": patch
"@radix-ui/react-switch": patch
"@radix-ui/react-tabs": patch
"@radix-ui/react-toast": patch
"@radix-ui/react-toggle": patch
"@radix-ui/react-toggle-group": patch
"@radix-ui/react-toolbar": patch
"@radix-ui/react-tooltip": patch
"@radix-ui/react-visually-hidden": patch

declined:
- primitives
- ssr-testing
33 changes: 19 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,34 @@
"bump:check": "yarn version check"
},
"dependencies": {
"react": "^18.3.1",
"react-dom": "^18.3.1"
"react": "^19.0.0",
"react-dom": "^19.0.0"
},
"devDependencies": {
"@stitches/core": "^1.2.8",
"@storybook/addon-essentials": "^7.6.17",
"@storybook/addon-storysource": "^7.6.17",
"@storybook/react": "^7.6.17",
"@storybook/react-webpack5": "^7.6.17",
"@storybook/test": "^7.6.17",
"@storybook/addon-essentials": "^8.4.7",
"@storybook/addon-storysource": "^8.4.7",
"@storybook/addon-webpack5-compiler-swc": "^1.0.5",
"@storybook/manager-api": "^8.4.7",
"@storybook/react": "^8.4.7",
"@storybook/react-webpack5": "^8.4.7",
"@storybook/test": "^8.4.7",
"@storybook/theming": "^8.4.7",
"@testing-library/cypress": "^7.0.6",
"@testing-library/dom": "^10.1.0",
"@testing-library/jest-dom": "^5.16.4",
"@testing-library/react": "^16.0.0",
"@testing-library/user-event": "^14.1.0",
"@types/eslint": "^7.28.0",
"@types/fs-extra": "^11",
"@types/jest": "^27.4.1",
"@types/jest": "^29.5.14",
"@types/jest-axe": "^3.5.3",
"@types/react": "^18.0.5",
"@types/react-dom": "^18.0.0",
"@types/react": "^19.0.1",
"@types/react-dom": "^19.0.2",
"@types/react-test-renderer": "^18.0.0",
"@typescript-eslint/eslint-plugin": "^5.19.0",
"@typescript-eslint/parser": "^5.19.0",
"babel-eslint": "^10.1.0",
"cypress": "^8.0.0",
"cypress-real-events": "^1.5.0",
"esbuild": "0.21.4",
Expand All @@ -67,19 +71,20 @@
"fs-extra": "^11.1.1",
"glob": "^10.2.2",
"husky": "^4.3.6",
"jest": "^27.5.1",
"jest": "^29.7.0",
"jest-axe": "^6.0.0",
"jest-environment-jsdom": "^29.7.0",
"jest-watch-typeahead": "^1.0.0",
"lint-staged": "^10.5.3",
"prettier": "^2.0.5",
"pretty-quick": "^2.0.1",
"react-test-renderer": "^18.0.0",
"replace-in-files": "^3.0.0",
"start-server-and-test": "2.0.3",
"storybook": "^7.6.17",
"ts-jest": "^27.1.4",
"storybook": "^8.4.7",
"ts-jest": "^29.2.5",
"tsup": "8.0.2",
"typescript": "^4.6.3"
"typescript": "^5.7.2"
},
"resolutions": {
"chokidar": "3.4.3"
Expand Down
2 changes: 1 addition & 1 deletion packages/core/primitive/src/primitive.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function composeEventHandlers<E>(
return function handleEvent(event: E) {
originalEventHandler?.(event);

if (checkForDefaultPrevented === false || !((event as unknown) as Event).defaultPrevented) {
if (checkForDefaultPrevented === false || !(event as unknown as Event).defaultPrevented) {
return ourEventHandler?.(event);
}
};
Expand Down
2 changes: 1 addition & 1 deletion packages/react/accessible-icon/src/AccessibleIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const AccessibleIcon: React.FC<AccessibleIconProps> = ({ children, label }) => {
const child = React.Children.only(children);
return (
<>
{React.cloneElement(child as React.ReactElement, {
{React.cloneElement(child as React.ReactElement<React.SVGAttributes<SVGElement>>, {
// accessibility
'aria-hidden': 'true',
focusable: 'false', // See: https://allyjs.io/tutorials/focusing-in-svg.html#making-svg-elements-focusable
Expand Down
2 changes: 1 addition & 1 deletion packages/react/alert-dialog/src/AlertDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ AlertDialogCancel.displayName = CANCEL_NAME;
/* ---------------------------------------------------------------------------------------------- */

type DescriptionWarningProps = {
contentRef: React.RefObject<AlertDialogContentElement>;
contentRef: React.RefObject<AlertDialogContentElement | null>;
};

const DescriptionWarning: React.FC<DescriptionWarningProps> = ({ contentRef }) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/collapsible/src/Collapsible.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ const CollapsibleContentImpl = React.forwardRef<
// when closing we delay `present` to retrieve dimensions before closing
const isOpen = context.open || isPresent;
const isMountAnimationPreventedRef = React.useRef(isOpen);
const originalStylesRef = React.useRef<Record<string, string>>();
const originalStylesRef = React.useRef<Record<string, string>>(undefined);

React.useEffect(() => {
const rAF = requestAnimationFrame(() => (isMountAnimationPreventedRef.current = false));
Expand Down
7 changes: 5 additions & 2 deletions packages/react/collection/src/Collection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ function createCollection<ItemElement extends HTMLElement, ItemData = {}>(name:
const [createCollectionContext, createCollectionScope] = createContextScope(PROVIDER_NAME);

type ContextValue = {
collectionRef: React.RefObject<CollectionElement>;
itemMap: Map<React.RefObject<ItemElement>, { ref: React.RefObject<ItemElement> } & ItemData>;
collectionRef: React.RefObject<CollectionElement | null>;
itemMap: Map<
React.RefObject<ItemElement | null>,
{ ref: React.RefObject<ItemElement | null> } & ItemData
>;
};

const [CollectionProviderImpl, useCollectionContext] = createCollectionContext<ContextValue>(
Expand Down
17 changes: 14 additions & 3 deletions packages/react/compose-refs/src/composeRefs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ type PossibleRef<T> = React.Ref<T> | undefined;
*/
function setRef<T>(ref: PossibleRef<T>, value: T) {
if (typeof ref === 'function') {
ref(value);
return ref(value);
} else if (ref !== null && ref !== undefined) {
(ref as React.MutableRefObject<T>).current = value;
ref.current = value;
}
}

Expand All @@ -19,7 +19,18 @@ function setRef<T>(ref: PossibleRef<T>, value: T) {
* Accepts callback refs and RefObject(s)
*/
function composeRefs<T>(...refs: PossibleRef<T>[]) {
return (node: T) => refs.forEach((ref) => setRef(ref, node));
return (node: T) => {
const cleanups = refs.map((ref) => setRef(ref, node));
return () => {
cleanups.forEach((cleanup, i) => {
if (typeof cleanup == 'function') {
cleanup();
} else {
setRef(refs[i], null);
}
});
};
};
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/react/dialog/src/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ type ScopedProps<P> = P & { __scopeDialog?: Scope };
const [createDialogContext, createDialogScope] = createContextScope(DIALOG_NAME);

type DialogContextValue = {
triggerRef: React.RefObject<HTMLButtonElement>;
contentRef: React.RefObject<DialogContentElement>;
triggerRef: React.RefObject<HTMLButtonElement | null>;
contentRef: React.RefObject<DialogContentElement | null>;
contentId: string;
titleId: string;
descriptionId: string;
Expand Down Expand Up @@ -524,7 +524,7 @@ For more information, see https://radix-ui.com/primitives/docs/components/${titl
const DESCRIPTION_WARNING_NAME = 'DialogDescriptionWarning';

type DescriptionWarningProps = {
contentRef: React.RefObject<DialogContentElement>;
contentRef: React.RefObject<DialogContentElement | null>;
descriptionId?: string;
};

Expand Down
2 changes: 1 addition & 1 deletion packages/react/dropdown-menu/src/DropdownMenu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ export const InPopupWindow = () => {
};

// change order slightly for more pleasing visual
const SIDES = SIDE_OPTIONS.filter((side) => side !== 'bottom').concat(['bottom']);
const SIDES = [...SIDE_OPTIONS.filter((side) => side !== 'bottom'), 'bottom' as const];

export const Chromatic = () => {
const checkboxItems = [
Expand Down
2 changes: 1 addition & 1 deletion packages/react/dropdown-menu/src/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const useMenuScope = createMenuScope();

type DropdownMenuContextValue = {
triggerId: string;
triggerRef: React.RefObject<HTMLButtonElement>;
triggerRef: React.RefObject<HTMLButtonElement | null>;
contentId: string;
open: boolean;
onOpenChange(open: boolean): void;
Expand Down
2 changes: 1 addition & 1 deletion packages/react/focus-scope/src/FocusScope.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const Multiple = () => {
};

// true => default focus, false => no focus, ref => focus element
type FocusParam = boolean | React.RefObject<HTMLElement>;
type FocusParam = boolean | React.RefObject<HTMLElement | null>;

export const WithOptions = () => {
const [open, setOpen] = React.useState(false);
Expand Down
2 changes: 1 addition & 1 deletion packages/react/hover-card/src/HoverCard.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ export const WithSlottedContent = () => (
);

// change order slightly for more pleasing visual
const SIDES = SIDE_OPTIONS.filter((side) => side !== 'bottom').concat(['bottom']);
const SIDES = [...SIDE_OPTIONS.filter((side) => side !== 'bottom'), 'bottom' as const];

export const Chromatic = () => (
<div style={{ padding: 200, paddingBottom: 500 }}>
Expand Down
2 changes: 1 addition & 1 deletion packages/react/menubar/src/Menubar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ const MENU_NAME = 'MenubarMenu';
type MenubarMenuContextValue = {
value: string;
triggerId: string;
triggerRef: React.RefObject<MenubarTriggerElement>;
triggerRef: React.RefObject<MenubarTriggerElement | null>;
contentId: string;
wasKeyboardTriggerOpenRef: React.MutableRefObject<boolean>;
};
Expand Down
10 changes: 5 additions & 5 deletions packages/react/navigation-menu/src/NavigationMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,9 @@ type FocusProxyElement = React.ElementRef<typeof VisuallyHiddenPrimitive.Root>;

type NavigationMenuItemContextValue = {
value: string;
triggerRef: React.RefObject<NavigationMenuTriggerElement>;
contentRef: React.RefObject<NavigationMenuContentElement>;
focusProxyRef: React.RefObject<FocusProxyElement>;
triggerRef: React.RefObject<NavigationMenuTriggerElement | null>;
contentRef: React.RefObject<NavigationMenuContentElement | null>;
focusProxyRef: React.RefObject<FocusProxyElement | null>;
wasEscapeCloseRef: React.MutableRefObject<boolean>;
onEntryKeyDown(): void;
onFocusProxyEnter(side: 'start' | 'end'): void;
Expand Down Expand Up @@ -833,8 +833,8 @@ type DismissableLayerProps = React.ComponentPropsWithoutRef<typeof DismissableLa

interface NavigationMenuContentImplPrivateProps {
value: string;
triggerRef: React.RefObject<NavigationMenuTriggerElement>;
focusProxyRef: React.RefObject<FocusProxyElement>;
triggerRef: React.RefObject<NavigationMenuTriggerElement | null>;
focusProxyRef: React.RefObject<FocusProxyElement | null>;
wasEscapeCloseRef: React.MutableRefObject<boolean>;
onContentFocusOutside(): void;
onRootContentClose(): void;
Expand Down
2 changes: 1 addition & 1 deletion packages/react/popover/src/Popover.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export const WithSlottedTrigger = () => {
};

// change order slightly for more pleasing visual
const SIDES = SIDE_OPTIONS.filter((side) => side !== 'bottom').concat(['bottom']);
const SIDES = [...SIDE_OPTIONS.filter((side) => side !== 'bottom'), 'bottom' as const];

export const Chromatic = () => (
<div style={{ padding: 200, paddingBottom: 500 }}>
Expand Down
2 changes: 1 addition & 1 deletion packages/react/popover/src/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const [createPopoverContext, createPopoverScope] = createContextScope(POPOVER_NA
const usePopperScope = createPopperScope();

type PopoverContextValue = {
triggerRef: React.RefObject<HTMLButtonElement>;
triggerRef: React.RefObject<HTMLButtonElement | null>;
contentId: string;
open: boolean;
onOpenChange(open: boolean): void;
Expand Down
4 changes: 2 additions & 2 deletions packages/react/presence/src/Presence.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const Presence: React.FC<PresenceProps> = (props) => {
typeof children === 'function'
? children({ present: presence.isPresent })
: React.Children.only(children)
) as React.ReactElement;
) as React.ReactElement<{ ref?: React.Ref<HTMLElement> }>;

const ref = useComposedRefs(presence.ref, getElementRef(child));
const forceMount = typeof children === 'function';
Expand Down Expand Up @@ -170,7 +170,7 @@ function getAnimationName(styles?: CSSStyleDeclaration) {
// https://github.com/facebook/react/pull/28348
//
// Access the ref using the method that doesn't yield a warning.
function getElementRef(element: React.ReactElement) {
function getElementRef(element: React.ReactElement<{ ref?: React.Ref<unknown> }>) {
// React <=18 in DEV
let getter = Object.getOwnPropertyDescriptor(element.props, 'ref')?.get;
let mayWarn = getter && 'isReactWarning' in getter && getter.isReactWarning;
Expand Down
2 changes: 1 addition & 1 deletion packages/react/scroll-area/src/ScrollArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ const ScrollAreaThumbImpl = React.forwardRef<ScrollAreaThumbImplElement, ScrollA
const composedRef = useComposedRefs(forwardedRef, (node) =>
scrollbarContext.onThumbChange(node)
);
const removeUnlinkedScrollListenerRef = React.useRef<() => void>();
const removeUnlinkedScrollListenerRef = React.useRef<() => void>(undefined);
const debounceScrollEnd = useDebounceCallback(() => {
if (removeUnlinkedScrollListenerRef.current) {
removeUnlinkedScrollListenerRef.current();
Expand Down
2 changes: 1 addition & 1 deletion packages/react/slider/src/Slider.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const Styled = () => (
</Slider.Root>
);

export const onValueCommit = () => (
export const WithOnValueCommit = () => (
<>
<Slider.Root className={rootClass()} defaultValue={[20]} onValueCommit={console.log}>
<Slider.Track className={trackClass()}>
Expand Down
4 changes: 2 additions & 2 deletions packages/react/slider/src/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ const SliderHorizontal = React.forwardRef<SliderHorizontalElement, SliderHorizon
} = props;
const [slider, setSlider] = React.useState<SliderImplElement | null>(null);
const composedRefs = useComposedRefs(forwardedRef, (node) => setSlider(node));
const rectRef = React.useRef<DOMRect>();
const rectRef = React.useRef<DOMRect>(undefined);
const direction = useDirection(dir);
const isDirectionLTR = direction === 'ltr';
const isSlidingFromLeft = (isDirectionLTR && !inverted) || (!isDirectionLTR && inverted);
Expand Down Expand Up @@ -326,7 +326,7 @@ const SliderVertical = React.forwardRef<SliderVerticalElement, SliderVerticalPro
} = props;
const sliderRef = React.useRef<SliderImplElement>(null);
const ref = useComposedRefs(forwardedRef, sliderRef);
const rectRef = React.useRef<DOMRect>();
const rectRef = React.useRef<DOMRect>(undefined);
const isSlidingFromBottom = !inverted;

function getValueFromPointer(pointerPosition: number) {
Expand Down
Loading

0 comments on commit 1b85e04

Please sign in to comment.