Skip to content

Commit 9a360c9

Browse files
authored
fix(ui): Extract ref from ExternalElementMounter props to prevent nodeRef overwrite (React 19) (#8534)
1 parent a1fc21c commit 9a360c9

3 files changed

Lines changed: 78 additions & 1 deletion

File tree

.changeset/soft-worms-battle.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@clerk/ui": patch
3+
---
4+
5+
Fixed custom page icons not rendering in React 19 due to a forwarded ref overwriting the internal node reference.

packages/ui/src/utils/ExternalElementMounter.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
1+
import type { Ref } from 'react';
12
import { useEffect, useRef } from 'react';
23

34
type ExternalElementMounterProps = {
45
mount: (el: HTMLDivElement) => void;
56
unmount: (el?: HTMLDivElement) => void;
7+
// In React 19, ref is a regular prop for function components (not wrapped in
8+
// forwardRef). Without this field, ref ends up in ...rest and overwrites the
9+
// internal nodeRef when spread onto the div, preventing mount from being called.
10+
ref?: Ref<HTMLDivElement>;
611
};
712

8-
export const ExternalElementMounter = ({ mount, unmount, ...rest }: ExternalElementMounterProps) => {
13+
export const ExternalElementMounter = ({
14+
mount,
15+
unmount,
16+
ref: _forwardedRef,
17+
...rest
18+
}: ExternalElementMounterProps) => {
919
const nodeRef = useRef(null);
1020

1121
useEffect(() => {
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { render, waitFor } from '@testing-library/react';
2+
import React from 'react';
3+
import { describe, expect, it, vi } from 'vitest';
4+
5+
import { ExternalElementMounter } from '../ExternalElementMounter';
6+
7+
describe('ExternalElementMounter', () => {
8+
it('calls mount with the host div element', async () => {
9+
const mount = vi.fn();
10+
const unmount = vi.fn();
11+
12+
render(
13+
<ExternalElementMounter
14+
mount={mount}
15+
unmount={unmount}
16+
/>,
17+
);
18+
19+
await waitFor(() => expect(mount).toHaveBeenCalledOnce());
20+
expect(mount).toHaveBeenCalledWith(expect.any(HTMLDivElement));
21+
});
22+
23+
it('calls unmount with the same element when removed', async () => {
24+
const mount = vi.fn();
25+
const unmount = vi.fn();
26+
27+
const { unmount: removeComponent } = render(
28+
<ExternalElementMounter
29+
mount={mount}
30+
unmount={unmount}
31+
/>,
32+
);
33+
34+
await waitFor(() => expect(mount).toHaveBeenCalledOnce());
35+
const mountedEl = mount.mock.calls[0][0];
36+
removeComponent();
37+
38+
expect(unmount).toHaveBeenCalledWith(mountedEl);
39+
});
40+
41+
it('calls mount even when a forwarded ref is present in props (React 19 regression)', async () => {
42+
// In React 19, ref is a regular prop. Before the fix, a ref forwarded through
43+
// the component chain would land in ...rest and overwrite nodeRef in
44+
// `<div ref={nodeRef} {...rest} />`, preventing mount from being called.
45+
// React 18 strips ref before passing to non-forwardRef components, so the
46+
// regression is only observable in a React 19 runtime — but this test guards
47+
// against it being reintroduced.
48+
const mount = vi.fn();
49+
const unmount = vi.fn();
50+
51+
render(
52+
React.createElement(ExternalElementMounter, {
53+
mount,
54+
unmount,
55+
ref: { current: null },
56+
}),
57+
);
58+
59+
await waitFor(() => expect(mount).toHaveBeenCalledOnce());
60+
expect(mount).toHaveBeenCalledWith(expect.any(HTMLDivElement));
61+
});
62+
});

0 commit comments

Comments
 (0)