From 3a5bf3f27e20e7a5c40024353b7fec8662f513d6 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 16 Nov 2023 19:34:57 +0100 Subject: [PATCH] chore(react-motions): improve error reporting on invalid elements (#29851) --- .../react-motions-preview/package.json | 3 +- .../src/factories/createAtom.ts | 3 +- .../src/factories/createTransition.ts | 3 +- .../src/utils/getChildElement.test.tsx | 37 +++++++++++++++++++ .../src/utils/getChildElement.ts | 24 ++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 packages/react-components/react-motions-preview/src/utils/getChildElement.test.tsx create mode 100644 packages/react-components/react-motions-preview/src/utils/getChildElement.ts diff --git a/packages/react-components/react-motions-preview/package.json b/packages/react-components/react-motions-preview/package.json index 02924f8ced3988..f2361117eead50 100644 --- a/packages/react-components/react-motions-preview/package.json +++ b/packages/react-components/react-motions-preview/package.json @@ -39,7 +39,8 @@ "dependencies": { "@fluentui/react-shared-contexts": "^9.12.0", "@fluentui/react-utilities": "^9.15.2", - "@swc/helpers": "^0.5.1" + "@swc/helpers": "^0.5.1", + "react-is": "^17.0.2" }, "peerDependencies": { "@types/react": ">=16.8.0 <19.0.0", diff --git a/packages/react-components/react-motions-preview/src/factories/createAtom.ts b/packages/react-components/react-motions-preview/src/factories/createAtom.ts index 06e8f190415678..d15f22b0451779 100644 --- a/packages/react-components/react-motions-preview/src/factories/createAtom.ts +++ b/packages/react-components/react-motions-preview/src/factories/createAtom.ts @@ -2,6 +2,7 @@ import { useIsomorphicLayoutEffect, useMergedRefs } from '@fluentui/react-utilit import * as React from 'react'; import { useIsReducedMotion } from '../hooks/useIsReducedMotion'; +import { getChildElement } from '../utils/getChildElement'; import type { MotionAtom } from '../types'; export type AtomProps = { @@ -20,7 +21,7 @@ export function createAtom(motion: MotionAtom) { const Atom: React.FC = props => { const { children, iterations = 1, playState = 'running' } = props; - const child = React.Children.only(children) as React.ReactElement & { ref: React.Ref }; + const child = getChildElement(children); const animationRef = React.useRef(); const elementRef = React.useRef(); diff --git a/packages/react-components/react-motions-preview/src/factories/createTransition.ts b/packages/react-components/react-motions-preview/src/factories/createTransition.ts index fd88352d9c6fb2..7c1fbd1fc7db2e 100644 --- a/packages/react-components/react-motions-preview/src/factories/createTransition.ts +++ b/packages/react-components/react-motions-preview/src/factories/createTransition.ts @@ -2,6 +2,7 @@ import { useEventCallback, useIsomorphicLayoutEffect, useMergedRefs } from '@flu import * as React from 'react'; import { useIsReducedMotion } from '../hooks/useIsReducedMotion'; +import { getChildElement } from '../utils/getChildElement'; import type { MotionTransition } from '../types'; type TransitionProps = { @@ -17,7 +18,7 @@ export function createTransition(transition: MotionTransition) { const Transition: React.FC = props => { const { appear, children, visible, unmountOnExit } = props; - const child = React.Children.only(children) as React.ReactElement & { ref: React.Ref }; + const child = getChildElement(children); const elementRef = React.useRef(); const ref = useMergedRefs(elementRef, child.ref); diff --git a/packages/react-components/react-motions-preview/src/utils/getChildElement.test.tsx b/packages/react-components/react-motions-preview/src/utils/getChildElement.test.tsx new file mode 100644 index 00000000000000..e00817db13a396 --- /dev/null +++ b/packages/react-components/react-motions-preview/src/utils/getChildElement.test.tsx @@ -0,0 +1,37 @@ +import * as React from 'react'; +import { getChildElement } from './getChildElement'; + +describe('getChildElement', () => { + it('throws if multiple elements are passed', () => { + expect(() => { + getChildElement([
,
] as unknown as React.ReactElement); + }).toThrow('@fluentui/react-motions: Invalid child element'); + }); + + it('throws if passed element does not support ref forwarding', () => { + const TestA = () =>
; + + // eslint-disable-next-line @typescript-eslint/naming-convention + function TestB() { + return
; + } + + expect(() => { + getChildElement(); + }).toThrow('@fluentui/react-motions: Invalid child element'); + expect(() => { + getChildElement(); + }).toThrow('@fluentui/react-motions: Invalid child element'); + }); + + it('does not throw if passed element does supports ref forwarding', () => { + const Test = React.forwardRef(() =>
); + + expect(() => { + getChildElement(); + }).not.toThrow(); + expect(() => { + getChildElement(
); + }).not.toThrow(); + }); +}); diff --git a/packages/react-components/react-motions-preview/src/utils/getChildElement.ts b/packages/react-components/react-motions-preview/src/utils/getChildElement.ts new file mode 100644 index 00000000000000..734871ccece395 --- /dev/null +++ b/packages/react-components/react-motions-preview/src/utils/getChildElement.ts @@ -0,0 +1,24 @@ +import * as React from 'react'; +import * as ReactIs from 'react-is'; + +export function getChildElement(children: React.ReactElement) { + try { + const child = React.Children.only(children) as React.ReactElement & { ref: React.Ref }; + + if (typeof child.type === 'string' || ReactIs.isForwardRef(child)) { + return child as React.ReactElement & { ref: React.Ref }; + } + + // We don't need to do anything here: we catch the exception from React to throw a more meaningful error + // eslint-disable-next-line no-empty + } catch {} + + throw new Error( + [ + '@fluentui/react-motions: Invalid child element.', + '\n', + '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().', + ].join(''), + ); +}