Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/quick-readers-cheer.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 3 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 27 additions & 0 deletions internal/test-utils/package.json
Original file line number Diff line number Diff line change
@@ -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": "*"
}
}
60 changes: 60 additions & 0 deletions internal/test-utils/ref-stability.tsx
Original file line number Diff line number Diff line change
@@ -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) => (
* <DismissableLayer.Root ref={ref}>
* <button type="button">inside</button>
* </DismissableLayer.Root>
* ));
*/
export function assertStableComposedRef(
renderWithRef: (ref: React.RefCallback<any>) => 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(<Probe />)).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);
}
9 changes: 9 additions & 0 deletions internal/test-utils/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "@repo/typescript-config/react-library.json",
"compilerOptions": {
"rootDir": ".",
"outDir": "dist"
},
"include": ["**/*"],
"exclude": ["node_modules", "dist"]
}
3 changes: 2 additions & 1 deletion packages/react/dismissable-layer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 10 additions & 0 deletions packages/react/dismissable-layer/src/dismissable-layer.test.tsx
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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) => (
<DismissableLayer.Root ref={ref}>
<button type="button">inside</button>
</DismissableLayer.Root>
));
});
});
23 changes: 18 additions & 5 deletions packages/react/dismissable-layer/src/dismissable-layer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -132,15 +132,28 @@ const DismissableLayer = React.forwardRef<DismissableLayerElement, DismissableLa
if (!event.defaultPrevented) onDismiss?.();
}, ownerDocument);

useEscapeKeydown((event) => {
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;
Expand Down
1 change: 1 addition & 0 deletions packages/react/dropdown-menu/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
40 changes: 40 additions & 0 deletions packages/react/dropdown-menu/src/dropdown-menu.test.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) => (
<DropdownMenu.Root>
<DropdownMenu.Trigger ref={ref}>{TRIGGER_TEXT}</DropdownMenu.Trigger>
<DropdownMenu.Portal>
<DropdownMenu.Content>
<DropdownMenu.Item>{ITEM_TEXT}</DropdownMenu.Item>
</DropdownMenu.Content>
</DropdownMenu.Portal>
</DropdownMenu.Root>
));
});

// Exercises the underlying `@radix-ui/react-menu` `MenuSubTrigger` fix.
it('keeps a stable composed ref on the SubTrigger', () => {
assertStableComposedRef((ref) => (
<DropdownMenu.Root defaultOpen>
<DropdownMenu.Trigger>{TRIGGER_TEXT}</DropdownMenu.Trigger>
<DropdownMenu.Portal>
<DropdownMenu.Content>
<DropdownMenu.Sub>
<DropdownMenu.SubTrigger ref={ref}>{SUB_TRIGGER_TEXT}</DropdownMenu.SubTrigger>
<DropdownMenu.Portal>
<DropdownMenu.SubContent>
<DropdownMenu.Item>{SUB_ITEM_TEXT}</DropdownMenu.Item>
</DropdownMenu.SubContent>
</DropdownMenu.Portal>
</DropdownMenu.Sub>
</DropdownMenu.Content>
</DropdownMenu.Portal>
</DropdownMenu.Root>
));
});
});
1 change: 1 addition & 0 deletions packages/react/focus-scope/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions packages/react/focus-scope/src/focus-scope.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) => (
<FocusScope asChild ref={ref}>
<button type="button">Click me</button>
</FocusScope>
));
});
});
});

function TestField({ label, ...props }: { label: string } & React.ComponentProps<'input'>) {
Expand Down
1 change: 1 addition & 0 deletions packages/react/navigation-menu/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
39 changes: 39 additions & 0 deletions packages/react/navigation-menu/src/navigation-menu.test.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) => (
<NavigationMenu.Root ref={ref}>
<NavigationMenu.List>
<NavigationMenu.Item value="one">
<NavigationMenu.Trigger>{TRIGGER_TEXT}</NavigationMenu.Trigger>
<NavigationMenu.Content>
<NavigationMenu.Link href="#">{CONTENT_TEXT}</NavigationMenu.Link>
</NavigationMenu.Content>
</NavigationMenu.Item>
</NavigationMenu.List>
<NavigationMenu.Viewport />
</NavigationMenu.Root>
));
});

// Exercises the viewport content item composed ref (`NavigationMenuViewportItem`).
it('keeps a stable composed ref on viewport content', () => {
assertStableComposedRef((ref) => (
<NavigationMenu.Root defaultValue="one">
<NavigationMenu.List>
<NavigationMenu.Item value="one">
<NavigationMenu.Trigger>{TRIGGER_TEXT}</NavigationMenu.Trigger>
<NavigationMenu.Content ref={ref}>
<NavigationMenu.Link href="#">{CONTENT_TEXT}</NavigationMenu.Link>
</NavigationMenu.Content>
</NavigationMenu.Item>
</NavigationMenu.List>
<NavigationMenu.Viewport />
</NavigationMenu.Root>
));
});
});
Loading
Loading