Skip to content

Commit dc43bc8

Browse files
authored
fix(react-motion): improve element validation to work with React 19 (#34164)
1 parent fd69b42 commit dc43bc8

File tree

8 files changed

+172
-78
lines changed

8 files changed

+172
-78
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: improve element validation to work with React 19",
4+
"packageName": "@fluentui/react-motion",
5+
"email": "olfedias@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-motion/library/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727
"dependencies": {
2828
"@fluentui/react-shared-contexts": "^9.23.1",
2929
"@fluentui/react-utilities": "^9.18.23",
30-
"@swc/helpers": "^0.5.1",
31-
"react-is": "^17.0.2"
30+
"@swc/helpers": "^0.5.1"
3231
},
3332
"peerDependencies": {
3433
"@types/react": ">=16.8.0 <19.0.0",

packages/react-components/react-motion/library/src/factories/createMotionComponent.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { useEventCallback, useIsomorphicLayoutEffect, useMergedRefs } from '@fluentui/react-utilities';
1+
import { useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
22
import * as React from 'react';
33

44
import { useAnimateAtoms } from '../hooks/useAnimateAtoms';
55
import { useMotionImperativeRef } from '../hooks/useMotionImperativeRef';
66
import { useIsReducedMotion } from '../hooks/useIsReducedMotion';
7-
import { getChildElement } from '../utils/getChildElement';
7+
import { useChildElement } from '../utils/useChildElement';
88
import type { AtomMotion, AtomMotionFn, MotionParam, MotionImperativeRef } from '../types';
99
import { useMotionBehaviourContext } from '../contexts/MotionBehaviourContext';
1010

@@ -63,10 +63,9 @@ export function createMotionComponent<MotionParams extends Record<string, Motion
6363
..._rest
6464
} = props;
6565
const params = _rest as Exclude<typeof props, MotionComponentProps>;
66-
const child = getChildElement(children);
66+
const [child, childRef] = useChildElement(children);
6767

6868
const handleRef = useMotionImperativeRef(imperativeRef);
69-
const elementRef = React.useRef<HTMLElement>();
7069
const skipMotions = useMotionBehaviourContext() === 'skip';
7170
const optionsRef = React.useRef<{ skipMotions: boolean; params: MotionParams }>({
7271
skipMotions,
@@ -95,7 +94,7 @@ export function createMotionComponent<MotionParams extends Record<string, Motion
9594
});
9695

9796
useIsomorphicLayoutEffect(() => {
98-
const element = elementRef.current;
97+
const element = childRef.current;
9998

10099
if (element) {
101100
const atoms = typeof value === 'function' ? value({ element, ...optionsRef.current.params }) : value;
@@ -113,9 +112,9 @@ export function createMotionComponent<MotionParams extends Record<string, Motion
113112
handle.cancel();
114113
};
115114
}
116-
}, [animateAtoms, handleRef, isReducedMotion, onMotionFinish, onMotionStart, onMotionCancel]);
115+
}, [animateAtoms, childRef, handleRef, isReducedMotion, onMotionFinish, onMotionStart, onMotionCancel]);
117116

118-
return React.cloneElement(children, { ref: useMergedRefs(elementRef, child.ref) });
117+
return child;
119118
};
120119

121120
return Atom;

packages/react-components/react-motion/library/src/factories/createPresenceComponent.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { useEventCallback, useFirstMount, useIsomorphicLayoutEffect, useMergedRefs } from '@fluentui/react-utilities';
1+
import { useEventCallback, useFirstMount, useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
22
import * as React from 'react';
33

44
import { PresenceGroupChildContext } from '../contexts/PresenceGroupChildContext';
55
import { useAnimateAtoms } from '../hooks/useAnimateAtoms';
66
import { useMotionImperativeRef } from '../hooks/useMotionImperativeRef';
77
import { useMountedState } from '../hooks/useMountedState';
88
import { useIsReducedMotion } from '../hooks/useIsReducedMotion';
9-
import { getChildElement } from '../utils/getChildElement';
9+
import { useChildElement } from '../utils/useChildElement';
1010
import type {
1111
MotionParam,
1212
PresenceMotion,
@@ -107,11 +107,9 @@ export function createPresenceComponent<MotionParams extends Record<string, Moti
107107
const params = _rest as Exclude<typeof merged, PresenceComponentProps | typeof itemContext>;
108108

109109
const [mounted, setMounted] = useMountedState(visible, unmountOnExit);
110-
const child = getChildElement(children);
110+
const [child, childRef] = useChildElement(children, mounted);
111111

112112
const handleRef = useMotionImperativeRef(imperativeRef);
113-
const elementRef = React.useRef<HTMLElement>();
114-
const ref = useMergedRefs(elementRef, child.ref);
115113
const optionsRef = React.useRef<{ appear?: boolean; params: MotionParams; skipMotions: boolean }>({
116114
appear,
117115
params,
@@ -146,7 +144,7 @@ export function createPresenceComponent<MotionParams extends Record<string, Moti
146144

147145
useIsomorphicLayoutEffect(
148146
() => {
149-
const element = elementRef.current;
147+
const element = childRef.current;
150148

151149
if (!element) {
152150
return;
@@ -223,11 +221,20 @@ export function createPresenceComponent<MotionParams extends Record<string, Moti
223221
},
224222
// Excluding `isFirstMount` from deps to prevent re-triggering the animation on subsequent renders
225223
// eslint-disable-next-line react-hooks/exhaustive-deps
226-
[animateAtoms, handleRef, isReducedMotion, handleMotionFinish, handleMotionStart, handleMotionCancel, visible],
224+
[
225+
animateAtoms,
226+
childRef,
227+
handleRef,
228+
isReducedMotion,
229+
handleMotionFinish,
230+
handleMotionStart,
231+
handleMotionCancel,
232+
visible,
233+
],
227234
);
228235

229236
if (mounted) {
230-
return React.cloneElement(child, { ref });
237+
return child;
231238
}
232239

233240
return null;

packages/react-components/react-motion/library/src/utils/getChildElement.test.tsx

Lines changed: 0 additions & 37 deletions
This file was deleted.

packages/react-components/react-motion/library/src/utils/getChildElement.ts

Lines changed: 0 additions & 24 deletions
This file was deleted.
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { render } from '@testing-library/react';
2+
import { renderHook } from '@testing-library/react-hooks';
3+
import * as React from 'react';
4+
5+
import { useChildElement } from './useChildElement';
6+
7+
const TestComponent: React.FC<{ children: React.ReactElement }> = ({ children }) => {
8+
const [child] = useChildElement(children);
9+
10+
return child;
11+
};
12+
13+
describe('useChildElement', () => {
14+
let consoleErrorSpy: jest.SpyInstance;
15+
16+
beforeAll(() => {
17+
// Silence React errors, we capture errors in the tests
18+
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {
19+
/* empty */
20+
});
21+
});
22+
23+
beforeEach(() => {
24+
jest.clearAllMocks();
25+
});
26+
27+
it('should return the child element and a ref to it', () => {
28+
const { result } = renderHook(() => useChildElement(<div />));
29+
30+
expect(result.current[0].type).toBe('div');
31+
expect(result.current[1]).toMatchObject({ current: null });
32+
});
33+
34+
it('should not log errors if a ref was assigned to the child', () => {
35+
const ComponentWithRef = React.forwardRef<HTMLDivElement>((props, ref) => <div ref={ref} {...props} />);
36+
37+
render(
38+
<TestComponent>
39+
<ComponentWithRef />
40+
</TestComponent>,
41+
);
42+
43+
expect(consoleErrorSpy).not.toHaveBeenCalled();
44+
});
45+
46+
it('should throw an error if no child is passed', () => {
47+
expect(() => render(<TestComponent children={undefined as unknown as React.ReactElement} />))
48+
.toThrowErrorMatchingInlineSnapshot(`
49+
"@fluentui/react-motion: Invalid child element.
50+
Motion factories require a single child element to be passed. That element element should support ref forwarding i.e. it should be either an intrinsic element (e.g. div) or a component that uses React.forwardRef()."
51+
`);
52+
expect(() => render(<TestComponent children={null as unknown as React.ReactElement} />))
53+
.toThrowErrorMatchingInlineSnapshot(`
54+
"@fluentui/react-motion: Invalid child element.
55+
Motion factories require a single child element to be passed. That element element should support ref forwarding i.e. it should be either an intrinsic element (e.g. div) or a component that uses React.forwardRef()."
56+
`);
57+
});
58+
59+
it('should throw an error if multiple children are passed', () => {
60+
expect(() =>
61+
render(<TestComponent children={[<div key="1" />, <div key="2" />] as unknown as React.ReactElement} />),
62+
).toThrowErrorMatchingInlineSnapshot(`
63+
"@fluentui/react-motion: Invalid child element.
64+
Motion factories require a single child element to be passed. That element element should support ref forwarding i.e. it should be either an intrinsic element (e.g. div) or a component that uses React.forwardRef()."
65+
`);
66+
});
67+
68+
it('should log an error if the child does not support ref forwarding', () => {
69+
const NonForwardingRefComponent: React.FC = () => {
70+
return null;
71+
};
72+
73+
render(
74+
<TestComponent>
75+
<NonForwardingRefComponent />
76+
</TestComponent>,
77+
);
78+
79+
expect(consoleErrorSpy).toHaveBeenCalledWith(
80+
expect.stringContaining('@fluentui/react-motion: Invalid child element.'),
81+
);
82+
});
83+
});
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import * as React from 'react';
2+
import { useMergedRefs } from '@fluentui/react-utilities';
3+
4+
const IS_REACT_19 = React.version.startsWith('19.');
5+
const CHILD_ERROR_MESSAGE = [
6+
'@fluentui/react-motion: Invalid child element.',
7+
'\n',
8+
'Motion factories require a single child element to be passed. ',
9+
'That element element should support ref forwarding i.e. it should be either an intrinsic element (e.g. div) or a component that uses React.forwardRef().',
10+
].join('');
11+
12+
/**
13+
* A backwards-compatible way to get the ref from a React element without console errors.
14+
*/
15+
function getRefFromReactElement<T>(element: React.ReactElement): React.Ref<T> | undefined {
16+
if (IS_REACT_19) {
17+
return element.props.ref;
18+
}
19+
20+
return (element as React.ReactElement & { ref: React.Ref<T> | undefined }).ref;
21+
}
22+
23+
/**
24+
* Validates the child and returns a cloned child element with a ref.
25+
*
26+
* Throws an error if the child is not a valid React element, similar to "React.Children.only".
27+
* Logs a warning in development mode if the ref is not set as the component remains functional.
28+
*/
29+
export function useChildElement(
30+
children: React.ReactElement,
31+
mounted: boolean = true,
32+
): [React.ReactElement, React.RefObject<HTMLElement>] {
33+
const childRef = React.useRef<HTMLElement>(null);
34+
35+
React.useEffect(() => {
36+
if (process.env.NODE_ENV !== 'production') {
37+
if (mounted && !childRef.current) {
38+
// eslint-disable-next-line no-console
39+
console.error(CHILD_ERROR_MESSAGE);
40+
}
41+
}
42+
}, [mounted]);
43+
44+
try {
45+
const child = React.Children.only(children) as Parameters<typeof React.isValidElement>[0];
46+
47+
if (React.isValidElement(child)) {
48+
return [
49+
React.cloneElement(child as React.ReactElement<{ ref: React.Ref<HTMLElement> }>, {
50+
ref: useMergedRefs(childRef, getRefFromReactElement(child)),
51+
}),
52+
childRef,
53+
];
54+
}
55+
} catch {
56+
/* empty */
57+
}
58+
59+
throw new Error(CHILD_ERROR_MESSAGE);
60+
}

0 commit comments

Comments
 (0)