diff --git a/.changeset/quick-readers-cheer.md b/.changeset/quick-readers-cheer.md new file mode 100644 index 0000000000..be3d392850 --- /dev/null +++ b/.changeset/quick-readers-cheer.md @@ -0,0 +1,16 @@ +--- +'@radix-ui/react-dismissable-layer': patch +'@radix-ui/react-dropdown-menu': patch +'@radix-ui/react-navigation-menu': patch +'@radix-ui/react-focus-scope': patch +'@radix-ui/react-scroll-area': patch +'@radix-ui/react-select': patch +'@radix-ui/react-popper': patch +'@radix-ui/react-slider': patch +'@radix-ui/react-toast': patch +'@radix-ui/react-menu': patch +'@radix-ui/react-use-escape-keydown': patch +--- + +- Fixed infinite re-render loop in React 19 caused by unstable composed ref callbacks being recreated on every render. +- Deprecated `useEscapeKeydown` in favor of attaching listeners directly via `useEffect` for more granular control over how callbacks are stabilized, when to detach listeners, etc. diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 671970a84a..d712824e36 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,6 +22,9 @@ jobs: - name: Lint run: pnpm lint + - name: Check composed refs + run: node scripts/check-composed-refs.mjs + - name: Run build run: pnpm build:ci diff --git a/internal/test-utils/package.json b/internal/test-utils/package.json new file mode 100644 index 0000000000..7c40cf1a03 --- /dev/null +++ b/internal/test-utils/package.json @@ -0,0 +1,27 @@ +{ + "name": "@repo/test-utils", + "version": "0.0.0", + "private": true, + "license": "MIT", + "type": "module", + "exports": { + "./*": "./*.tsx" + }, + "scripts": { + "lint": "oxlint --max-warnings 0 .", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "@repo/typescript-config": "workspace:*", + "@testing-library/react": "^16.3.2", + "@types/react": "^19.2.15", + "react": "^19.2.6", + "typescript": "^5.9.3", + "vitest": "^4.1.7" + }, + "peerDependencies": { + "@testing-library/react": "*", + "react": "*", + "vitest": "*" + } +} diff --git a/internal/test-utils/ref-stability.tsx b/internal/test-utils/ref-stability.tsx new file mode 100644 index 0000000000..a931671798 --- /dev/null +++ b/internal/test-utils/ref-stability.tsx @@ -0,0 +1,60 @@ +import * as React from 'react'; +import { render } from '@testing-library/react'; +import { expect } from 'vitest'; + +const DEFAULT_MAX_RENDERS = 25; + +interface AssertStableComposedRefOptions { + /** + * The render count at which we consider the component to be stuck in a render + * loop. Defaults to {@link DEFAULT_MAX_RENDERS}. + */ + maxRenders?: number; +} + +/** + * Regression guard for unstable composed refs. + * See https://github.com/radix-ui/primitives/issues/3963. + * + * Renders the component under test with a *stable* callback ref that forces a + * re-render the first time it receives a node. If the component composes that + * forwarded ref with an unstable member ā€” e.g. an inline + * `useComposedRefs(ref, (node) => setX(node))` or a render-inline + * `composeRefs(...)` used directly as a `ref` ā€” React 19 detaches and + * re-attaches the ref on every commit. That re-runs our forced render forever + * and throws "Maximum update depth exceeded". A correctly stabilized composed + * ref settles after a couple of renders. + * + * @example + * assertStableComposedRef((ref) => ( + * + * + * + * )); + */ +export function assertStableComposedRef( + renderWithRef: (ref: React.RefCallback) => React.ReactElement, + { maxRenders = DEFAULT_MAX_RENDERS }: AssertStableComposedRefOptions = {}, +) { + let renderCount = 0; + + function Probe() { + renderCount++; + const [, forceRender] = React.useReducer(() => Object.create(null), null); + // The probe ref is intentionally stable (empty deps) so that any render loop + // is caused by the component under test, not by this helper. + const ref = React.useCallback((node: unknown) => { + if (node) forceRender(); + }, []); + return renderWithRef(ref); + } + + expect(() => render()).not.toThrow(); + expect( + renderCount, + `Expected the component to settle, but it rendered ${renderCount} times ` + + `(>= ${maxRenders}). This usually means a composed ref callback is being ` + + `recreated on every render. Pass stable refs (e.g. state setters) directly ` + + `to \`useComposedRefs\`, or wrap inline callbacks in \`useCallbackRef\`.`, + ).toBeLessThan(maxRenders); +} diff --git a/internal/test-utils/tsconfig.json b/internal/test-utils/tsconfig.json new file mode 100644 index 0000000000..470a2d6902 --- /dev/null +++ b/internal/test-utils/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "@repo/typescript-config/react-library.json", + "compilerOptions": { + "rootDir": ".", + "outDir": "dist" + }, + "include": ["**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/react/dismissable-layer/package.json b/packages/react/dismissable-layer/package.json index 70e1518df1..70afab8365 100644 --- a/packages/react/dismissable-layer/package.json +++ b/packages/react/dismissable-layer/package.json @@ -39,10 +39,11 @@ "@radix-ui/react-compose-refs": "workspace:*", "@radix-ui/react-primitive": "workspace:*", "@radix-ui/react-use-callback-ref": "workspace:*", - "@radix-ui/react-use-escape-keydown": "workspace:*" + "@radix-ui/react-use-effect-event": "workspace:*" }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/dismissable-layer/src/dismissable-layer.test.tsx b/packages/react/dismissable-layer/src/dismissable-layer.test.tsx index 4ad5769f49..9354601305 100644 --- a/packages/react/dismissable-layer/src/dismissable-layer.test.tsx +++ b/packages/react/dismissable-layer/src/dismissable-layer.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as DismissableLayer from './dismissable-layer'; async function waitForDocumentPointerDownListener() { @@ -395,4 +396,13 @@ describe('DismissableLayer', () => { expect(onDismiss).not.toHaveBeenCalled(); }); + + // Regression test for https://github.com/radix-ui/primitives/issues/3963 + it('keeps a stable composed ref (no infinite render loop)', () => { + assertStableComposedRef((ref) => ( + + + + )); + }); }); diff --git a/packages/react/dismissable-layer/src/dismissable-layer.tsx b/packages/react/dismissable-layer/src/dismissable-layer.tsx index 6e5503ce94..e607af2154 100644 --- a/packages/react/dismissable-layer/src/dismissable-layer.tsx +++ b/packages/react/dismissable-layer/src/dismissable-layer.tsx @@ -3,7 +3,7 @@ import { composeEventHandlers } from '@radix-ui/primitive'; import { Primitive, dispatchDiscreteCustomEvent } from '@radix-ui/react-primitive'; import { useComposedRefs } from '@radix-ui/react-compose-refs'; import { useCallbackRef } from '@radix-ui/react-use-callback-ref'; -import { useEscapeKeydown } from '@radix-ui/react-use-escape-keydown'; +import { useEffectEvent } from '@radix-ui/react-use-effect-event'; /* ------------------------------------------------------------------------------------------------- * DismissableLayer @@ -132,15 +132,28 @@ const DismissableLayer = React.forwardRef { - const isHighestLayer = index === context.layers.size - 1; - if (!isHighestLayer) return; + const isHighestLayer = node ? index === layers.length - 1 : false; + + const handleKeyDown = useEffectEvent((event: KeyboardEvent) => { + if (event.key !== 'Escape') { + return; + } + onEscapeKeyDown?.(event); if (!event.defaultPrevented && onDismiss) { event.preventDefault(); onDismiss(); } - }, ownerDocument); + }); + + React.useEffect(() => { + if (!isHighestLayer) { + return; + } + + ownerDocument.addEventListener('keydown', handleKeyDown, { capture: true }); + return () => ownerDocument.removeEventListener('keydown', handleKeyDown, { capture: true }); + }, [ownerDocument, isHighestLayer]); React.useEffect(() => { if (!node) return; diff --git a/packages/react/dropdown-menu/package.json b/packages/react/dropdown-menu/package.json index da460078f0..2f867d9ccb 100644 --- a/packages/react/dropdown-menu/package.json +++ b/packages/react/dropdown-menu/package.json @@ -46,6 +46,7 @@ "devDependencies": { "@repo/builder": "workspace:*", "@repo/test-data": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/dropdown-menu/src/dropdown-menu.test.tsx b/packages/react/dropdown-menu/src/dropdown-menu.test.tsx index c6ab867313..c7431a0484 100644 --- a/packages/react/dropdown-menu/src/dropdown-menu.test.tsx +++ b/packages/react/dropdown-menu/src/dropdown-menu.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { afterEach, describe, it, expect } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as DropdownMenu from './dropdown-menu'; const TRIGGER_TEXT = 'Open'; @@ -84,3 +85,42 @@ describe('closing on window blur', () => { expect(trigger).toHaveAttribute('aria-expanded', 'false'); }); }); + +// Regression tests for https://github.com/radix-ui/primitives/issues/3963 +describe('ref stability', () => { + afterEach(cleanup); + + it('keeps a stable composed ref on the Trigger', () => { + assertStableComposedRef((ref) => ( + + {TRIGGER_TEXT} + + + {ITEM_TEXT} + + + + )); + }); + + // Exercises the underlying `@radix-ui/react-menu` `MenuSubTrigger` fix. + it('keeps a stable composed ref on the SubTrigger', () => { + assertStableComposedRef((ref) => ( + + {TRIGGER_TEXT} + + + + {SUB_TRIGGER_TEXT} + + + {SUB_ITEM_TEXT} + + + + + + + )); + }); +}); diff --git a/packages/react/focus-scope/package.json b/packages/react/focus-scope/package.json index fcbc1e230e..b02a3d94a6 100644 --- a/packages/react/focus-scope/package.json +++ b/packages/react/focus-scope/package.json @@ -41,6 +41,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/focus-scope/src/focus-scope.test.tsx b/packages/react/focus-scope/src/focus-scope.test.tsx index 10c448843f..9dbb2ac6fe 100644 --- a/packages/react/focus-scope/src/focus-scope.test.tsx +++ b/packages/react/focus-scope/src/focus-scope.test.tsx @@ -4,6 +4,7 @@ import { cleanup, render, waitFor } from '@testing-library/react'; import { FocusScope } from './focus-scope'; import type { RenderResult } from '@testing-library/react'; import { afterEach, describe, it, beforeEach, vi, expect } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; const INNER_NAME_INPUT_LABEL = 'Name'; const INNER_EMAIL_INPUT_LABEL = 'Email'; @@ -118,6 +119,17 @@ describe('FocusScope', () => { await waitFor(() => expect(handleLastFocusableElementBlur).toHaveBeenCalledTimes(1)); }); }); + + // Regression test for https://github.com/radix-ui/primitives/issues/3963 + describe('ref stability', () => { + it('keeps a stable composed ref (no infinite render loop)', () => { + assertStableComposedRef((ref) => ( + + + + )); + }); + }); }); function TestField({ label, ...props }: { label: string } & React.ComponentProps<'input'>) { diff --git a/packages/react/navigation-menu/package.json b/packages/react/navigation-menu/package.json index 8679a31b98..ede6b166b4 100644 --- a/packages/react/navigation-menu/package.json +++ b/packages/react/navigation-menu/package.json @@ -52,6 +52,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/navigation-menu/src/navigation-menu.test.tsx b/packages/react/navigation-menu/src/navigation-menu.test.tsx index 3bbffd07bf..a315fce9ae 100644 --- a/packages/react/navigation-menu/src/navigation-menu.test.tsx +++ b/packages/react/navigation-menu/src/navigation-menu.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { cleanup, render, screen, waitFor } from '@testing-library/react'; import { afterEach, describe, it, expect } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as NavigationMenu from './navigation-menu'; const TRIGGER_TEXT = 'Item One'; @@ -44,3 +45,41 @@ describe('aria-controls', () => { expect(content).toContainElement(screen.getByText(CONTENT_TEXT)); }); }); + +// Regression tests for https://github.com/radix-ui/primitives/issues/3963 +describe('NavigationMenu ref stability', () => { + afterEach(cleanup); + + it('keeps a stable composed ref on the root', () => { + assertStableComposedRef((ref) => ( + + + + {TRIGGER_TEXT} + + {CONTENT_TEXT} + + + + + + )); + }); + + // Exercises the viewport content item composed ref (`NavigationMenuViewportItem`). + it('keeps a stable composed ref on viewport content', () => { + assertStableComposedRef((ref) => ( + + + + {TRIGGER_TEXT} + + {CONTENT_TEXT} + + + + + + )); + }); +}); diff --git a/packages/react/navigation-menu/src/navigation-menu.tsx b/packages/react/navigation-menu/src/navigation-menu.tsx index b9911bd24d..6d90ca38ba 100644 --- a/packages/react/navigation-menu/src/navigation-menu.tsx +++ b/packages/react/navigation-menu/src/navigation-menu.tsx @@ -4,7 +4,7 @@ import { createContextScope } from '@radix-ui/react-context'; import { composeEventHandlers } from '@radix-ui/primitive'; import { Primitive, dispatchDiscreteCustomEvent } from '@radix-ui/react-primitive'; import { useControllableState } from '@radix-ui/react-use-controllable-state'; -import { composeRefs, useComposedRefs } from '@radix-ui/react-compose-refs'; +import { useComposedRefs } from '@radix-ui/react-compose-refs'; import { useDirection } from '@radix-ui/react-direction'; import { Presence } from '@radix-ui/react-presence'; import { useId } from '@radix-ui/react-id'; @@ -110,7 +110,7 @@ const NavigationMenu = React.forwardRef(null); - const composedRef = useComposedRefs(forwardedRef, (node) => setNavigationMenu(node)); + const composedRef = useComposedRefs(forwardedRef, setNavigationMenu); const direction = useDirection(dir); const openTimerRef = React.useRef(0); const closeTimerRef = React.useRef(0); @@ -1070,13 +1070,11 @@ const NavigationMenuViewportImpl = React.forwardRef< const isActive = activeContentValue === value; return ( - { - // We only want to update the stored node when another is available - // as we need to smoothly transition between them. - if (isActive && node) setContent(node); - })} + contentRef={ref} + isActive={isActive} + onActiveContentChange={setContent} /> ); @@ -1087,6 +1085,34 @@ const NavigationMenuViewportImpl = React.forwardRef< /* -----------------------------------------------------------------------------------------------*/ +interface NavigationMenuViewportItemProps extends NavigationMenuContentImplProps { + contentRef?: React.Ref; + isActive: boolean; + onActiveContentChange: (node: NavigationMenuContentElement | null) => void; +} + +const NavigationMenuViewportItem = ({ + contentRef, + isActive, + onActiveContentChange, + ...props +}: ScopedProps) => { + const handleContentChange = React.useCallback( + (node: NavigationMenuContentElement | null) => { + // We only want to update the stored node when another is available + // as we need to smoothly transition between them. + if (isActive && node) { + onActiveContentChange(node); + } + }, + [isActive, onActiveContentChange], + ); + const composedRefs = useComposedRefs(contentRef, handleContentChange); + return ; +}; + +/* -----------------------------------------------------------------------------------------------*/ + const FOCUS_GROUP_NAME = 'FocusGroup'; type FocusGroupElement = React.ComponentRef; diff --git a/packages/react/popper/package.json b/packages/react/popper/package.json index 4f029ab26b..4b16f8792e 100644 --- a/packages/react/popper/package.json +++ b/packages/react/popper/package.json @@ -48,6 +48,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/popper/src/popper.test.tsx b/packages/react/popper/src/popper.test.tsx index 76aaa6841d..fd8bfd6db3 100644 --- a/packages/react/popper/src/popper.test.tsx +++ b/packages/react/popper/src/popper.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { cleanup, render, act } from '@testing-library/react'; import { afterEach, describe, it, expect, vi } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as Popper from './popper'; function makeVirtual() { @@ -84,3 +85,17 @@ describe('PopperAnchor virtualRef', () => { expect(anchor.getBoundingClientRect).toHaveBeenCalled(); }); }); + +// Regression test for https://github.com/radix-ui/primitives/issues/3963 +describe('PopperContent ref stability', () => { + afterEach(cleanup); + + it('keeps a stable composed ref (no infinite render loop)', () => { + assertStableComposedRef((ref) => ( + + + content + + )); + }); +}); diff --git a/packages/react/scroll-area/package.json b/packages/react/scroll-area/package.json index 8b21163aea..b8f1af0c6d 100644 --- a/packages/react/scroll-area/package.json +++ b/packages/react/scroll-area/package.json @@ -47,6 +47,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/scroll-area/src/scroll-area.test.tsx b/packages/react/scroll-area/src/scroll-area.test.tsx new file mode 100644 index 0000000000..df130646df --- /dev/null +++ b/packages/react/scroll-area/src/scroll-area.test.tsx @@ -0,0 +1,29 @@ +import * as React from 'react'; +import { afterEach, describe, it } from 'vitest'; +import { cleanup } from '@testing-library/react'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; +import * as ScrollArea from './scroll-area'; + +// Regression tests for https://github.com/radix-ui/primitives/issues/3963 +describe('ScrollArea ref stability', () => { + afterEach(cleanup); + + it('keeps a stable composed ref on the root', () => { + assertStableComposedRef((ref) => ( + + content + + )); + }); + + it('keeps a stable composed ref on the scrollbar', () => { + assertStableComposedRef((ref) => ( + + content + + + + + )); + }); +}); diff --git a/packages/react/select/package.json b/packages/react/select/package.json index 30c0c41f3d..cd83bb8559 100644 --- a/packages/react/select/package.json +++ b/packages/react/select/package.json @@ -61,6 +61,7 @@ "devDependencies": { "@repo/builder": "workspace:*", "@repo/test-data": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/select/src/select.test.tsx b/packages/react/select/src/select.test.tsx index 6a140906cd..834f702412 100644 --- a/packages/react/select/src/select.test.tsx +++ b/packages/react/select/src/select.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { afterEach, describe, it, expect } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as Select from './select'; const PLACEHOLDER_TEXT = 'Pick one'; @@ -140,3 +141,38 @@ describe('clearing an optional value (#2706)', () => { expect(emptyOptions).toHaveLength(1); }); }); + +// Regression tests for https://github.com/radix-ui/primitives/issues/3963 +describe('Select ref stability', () => { + afterEach(cleanup); + + const renderOpenSelect = + (slot: 'content' | 'item' | 'itemText') => (ref: React.RefCallback) => ( + + + + + + + + + Apple + + + + + + ); + + it('keeps a stable composed ref on Content', () => { + assertStableComposedRef(renderOpenSelect('content')); + }); + + it('keeps a stable composed ref on Item', () => { + assertStableComposedRef(renderOpenSelect('item')); + }); + + it('keeps a stable composed ref on ItemText', () => { + assertStableComposedRef(renderOpenSelect('itemText')); + }); +}); diff --git a/packages/react/select/src/select.tsx b/packages/react/select/src/select.tsx index 2389471b19..3f7c65993a 100644 --- a/packages/react/select/src/select.tsx +++ b/packages/react/select/src/select.tsx @@ -1367,7 +1367,7 @@ const SelectItem = React.forwardRef( const [textValue, setTextValue] = React.useState(textValueProp ?? ''); const [isFocused, setIsFocused] = React.useState(false); const handleItemRefCallback = useCallbackRef((node: SelectItemElement | null) => - contentContext.itemRefCallback?.(node, value, disabled) + contentContext.itemRefCallback?.(node, value, disabled), ); const composedRefs = useComposedRefs(forwardedRef, handleItemRefCallback); const textId = useId(); @@ -1474,7 +1474,7 @@ const SelectItemText = React.forwardRef(null); const handleItemTextRefCallback = useCallbackRef((node: SelectItemTextElement | null) => - contentContext.itemTextRefCallback?.(node, itemContext.value, itemContext.disabled) + contentContext.itemTextRefCallback?.(node, itemContext.value, itemContext.disabled), ); const composedRefs = useComposedRefs( forwardedRef, diff --git a/packages/react/slider/package.json b/packages/react/slider/package.json index 33c110f0bb..5d0cb3c68c 100644 --- a/packages/react/slider/package.json +++ b/packages/react/slider/package.json @@ -49,6 +49,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/slider/src/slider.test.tsx b/packages/react/slider/src/slider.test.tsx index 914060d618..fc66c899ec 100644 --- a/packages/react/slider/src/slider.test.tsx +++ b/packages/react/slider/src/slider.test.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import { cleanup, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { afterEach, describe, it, expect } from 'vitest'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; import * as Slider from './slider'; function renderSlider(props: React.ComponentProps) { @@ -54,4 +55,29 @@ describe('Slider', () => { expect(getThumbValue()).toBeCloseTo(1.5e-7, 12); }); + + // Regression tests for https://github.com/radix-ui/primitives/issues/3963 + describe('ref stability', () => { + it('keeps a stable composed ref on the root', () => { + assertStableComposedRef((ref) => ( + + + + + + + )); + }); + + it('keeps a stable composed ref on the thumb', () => { + assertStableComposedRef((ref) => ( + + + + + + + )); + }); + }); }); diff --git a/packages/react/slider/src/slider.tsx b/packages/react/slider/src/slider.tsx index ecdaf3738b..5ec0668a03 100644 --- a/packages/react/slider/src/slider.tsx +++ b/packages/react/slider/src/slider.tsx @@ -279,7 +279,7 @@ const SliderHorizontal = React.forwardRef(null); - const composedRefs = useComposedRefs(forwardedRef, (node) => setSlider(node)); + const composedRefs = useComposedRefs(forwardedRef, setSlider); const rectRef = React.useRef(undefined); const direction = useDirection(dir); const isDirectionLTR = direction === 'ltr'; @@ -660,7 +660,7 @@ const SliderThumbTrigger = React.forwardRef onThumbChange(node)); + const composedRefs = useComposedRefs(forwardedRef, onThumbChange); const label = getLabel(index, context.values.length); const orientationSize = size?.[orientation.size]; const thumbInBoundsOffset = orientationSize diff --git a/packages/react/toast/package.json b/packages/react/toast/package.json index 57a329cad6..4ab10cd665 100644 --- a/packages/react/toast/package.json +++ b/packages/react/toast/package.json @@ -50,6 +50,7 @@ }, "devDependencies": { "@repo/builder": "workspace:*", + "@repo/test-utils": "workspace:*", "@repo/typescript-config": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.2", diff --git a/packages/react/toast/src/toast.test.tsx b/packages/react/toast/src/toast.test.tsx new file mode 100644 index 0000000000..c224a6a929 --- /dev/null +++ b/packages/react/toast/src/toast.test.tsx @@ -0,0 +1,21 @@ +import * as React from 'react'; +import { afterEach, describe, it } from 'vitest'; +import { cleanup } from '@testing-library/react'; +import { assertStableComposedRef } from '@repo/test-utils/ref-stability'; +import * as Toast from './toast'; + +// Regression test for https://github.com/radix-ui/primitives/issues/3963 +describe('Toast ref stability', () => { + afterEach(cleanup); + + it('keeps a stable composed ref on the Toast root', () => { + assertStableComposedRef((ref) => ( + + + Title + + + + )); + }); +}); diff --git a/packages/react/toast/src/toast.tsx b/packages/react/toast/src/toast.tsx index 0b71d0d712..b5d62ec378 100644 --- a/packages/react/toast/src/toast.tsx +++ b/packages/react/toast/src/toast.tsx @@ -490,7 +490,7 @@ const ToastImpl = React.forwardRef( } = props; const context = useToastProviderContext(TOAST_NAME, __scopeToast); const [node, setNode] = React.useState(null); - const composedRefs = useComposedRefs(forwardedRef, (node) => setNode(node)); + const composedRefs = useComposedRefs(forwardedRef, setNode); const pointerStartRef = React.useRef<{ x: number; y: number } | null>(null); const swipeDeltaRef = React.useRef<{ x: number; y: number } | null>(null); const duration = durationProp || context.duration; diff --git a/packages/react/use-escape-keydown/src/use-escape-keydown.tsx b/packages/react/use-escape-keydown/src/use-escape-keydown.tsx index e93f90d781..391db39636 100644 --- a/packages/react/use-escape-keydown/src/use-escape-keydown.tsx +++ b/packages/react/use-escape-keydown/src/use-escape-keydown.tsx @@ -3,6 +3,9 @@ import { useCallbackRef } from '@radix-ui/react-use-callback-ref'; /** * Listens for when the escape key is down + * + * @deprecated Attach listeners directly via useEffect for more granular control + * over how callbacks are stabilized, when to detach listeners, etc. */ function useEscapeKeydown( onEscapeKeyDownProp?: (event: KeyboardEvent) => void, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3143dd54d5..9da291b119 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -222,6 +222,27 @@ importers: specifier: ^5.9.3 version: 5.9.3 + internal/test-utils: + devDependencies: + '@repo/typescript-config': + specifier: workspace:* + version: link:../typescript-config + '@testing-library/react': + specifier: ^16.3.2 + version: 16.3.2(@testing-library/dom@10.4.1)(@types/react-dom@19.2.3(@types/react@19.2.15))(@types/react@19.2.15)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) + '@types/react': + specifier: ^19.2.15 + version: 19.2.15 + react: + specifier: ^19.2.6 + version: 19.2.6 + typescript: + specifier: ^5.9.3 + version: 5.9.3 + vitest: + specifier: ^4.1.7 + version: 4.1.7(@types/node@24.12.4)(jsdom@29.1.1)(vite@7.3.3(@types/node@24.12.4)(terser@5.48.0)) + internal/typescript-config: {} packages/core/number: @@ -841,13 +862,16 @@ importers: '@radix-ui/react-use-callback-ref': specifier: workspace:* version: link:../use-callback-ref - '@radix-ui/react-use-escape-keydown': + '@radix-ui/react-use-effect-event': specifier: workspace:* - version: link:../use-escape-keydown + version: link:../use-effect-event devDependencies: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -900,6 +924,9 @@ importers: '@repo/test-data': specifier: workspace:* version: link:../../../internal/test-data + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -958,6 +985,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -1316,6 +1346,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -1551,6 +1584,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -2018,6 +2054,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -2112,6 +2151,9 @@ importers: '@repo/test-data': specifier: workspace:* version: link:../../../internal/test-data + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -2198,6 +2240,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config @@ -2382,6 +2427,9 @@ importers: '@repo/builder': specifier: workspace:* version: link:../../../internal/builder + '@repo/test-utils': + specifier: workspace:* + version: link:../../../internal/test-utils '@repo/typescript-config': specifier: workspace:* version: link:../../../internal/typescript-config diff --git a/scripts/check-composed-refs.mjs b/scripts/check-composed-refs.mjs new file mode 100644 index 0000000000..df3942694f --- /dev/null +++ b/scripts/check-composed-refs.mjs @@ -0,0 +1,123 @@ +// @ts-check +/** + * Guards against reintroducing the unstable composed-ref footgun that caused the + * React 19 "Maximum update depth exceeded" crashes in issue #3963. + * + * It fails CI when it finds either of these anti-patterns in package source: + * + * 1. An inline function passed to `useComposedRefs(...)`, e.g. + * useComposedRefs(forwardedRef, (node) => setContent(node)) + * The inline callback is recreated every render, so the memoized composed + * ref changes identity every render. Pass the stable ref (e.g. the state + * setter) directly, or wrap the callback in `useCallbackRef` first. + * + * 2. A render-inline `composeRefs(...)` used as a `ref`, e.g. + * + * This builds a brand new ref callback every render. Hoist it into the + * memoized `useComposedRefs(...)` hook instead. + */ +import { globSync } from 'glob'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; +import ts from 'typescript'; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +const files = globSync('packages/*/*/src/**/*.{ts,tsx}', { + cwd: root, + absolute: true, + ignore: ['**/*.test.*', '**/*.stories.*', '**/dist/**', '**/node_modules/**'], +}); + +/** @type {{ file: string; line: number; message: string }[]} */ +const violations = []; + +/** + * @param {ts.SourceFile} sourceFile + * @param {string} file + * @param {ts.Node} node + * @param {string} message + */ +function report(sourceFile, file, node, message) { + const { line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)); + violations.push({ file: path.relative(root, file), line: line + 1, message }); +} + +const INLINE_USE_COMPOSED_REFS = + 'Inline ref callback passed to `useComposedRefs`. Pass a stable ref (e.g. a state setter) ' + + 'directly, or wrap the callback in `useCallbackRef` before composing (see issue #3963).'; + +const RENDER_INLINE_COMPOSE_REFS = + 'Render-inline `composeRefs(...)` used as a ref. This recreates the ref on every render; ' + + 'hoist it into the memoized `useComposedRefs(...)` hook instead (see issue #3963).'; + +/** + * @param {ts.Node} node + * @returns {node is ts.CallExpression} + */ +function isCallTo(node, name) { + return ( + ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === name + ); +} + +for (const file of files) { + const sourceFile = ts.createSourceFile( + file, + readFileSync(file, 'utf8'), + ts.ScriptTarget.Latest, + /* setParentNodes */ true, + ts.ScriptKind.TSX, + ); + + /** @param {ts.Node} node */ + const findComposeRefs = (node) => { + if (isCallTo(node, 'composeRefs')) { + report(sourceFile, file, node, RENDER_INLINE_COMPOSE_REFS); + } + ts.forEachChild(node, findComposeRefs); + }; + + /** @param {ts.Node} node */ + const visit = (node) => { + if (isCallTo(node, 'useComposedRefs')) { + for (const arg of node.arguments) { + if (ts.isArrowFunction(arg) || ts.isFunctionExpression(arg)) { + report(sourceFile, file, arg, INLINE_USE_COMPOSED_REFS); + } + } + } + + // Flag `composeRefs(...)` used anywhere inside a JSX attribute (e.g. `ref={...}`). + if ( + ts.isJsxAttribute(node) && + node.initializer && + ts.isJsxExpression(node.initializer) && + node.initializer.expression + ) { + findComposeRefs(node.initializer.expression); + } + + ts.forEachChild(node, visit); + }; + + visit(sourceFile); +} + +if (violations.length > 0) { + console.error(`\nāœ– Found ${violations.length} unstable composed-ref pattern(s):\n`); + for (const violation of violations) { + console.error(` ${violation.file}:${violation.line}`); + console.error(` ${violation.message}\n`); + } + console.error( + 'These patterns cause React 19 to detach/re-attach refs every commit, which can trigger\n' + + 'infinite render loops ("Maximum update depth exceeded"). See issue #3963.\n', + ); + process.exit(1); +} + +console.log( + `āœ“ check-composed-refs: no unstable composed-ref patterns found (${files.length} files).`, +);