diff --git a/change/@fluentui-react-motion-preview-20cc46cf-11ec-49a9-9f37-0e1385ab3b40.json b/change/@fluentui-react-motion-preview-20cc46cf-11ec-49a9-9f37-0e1385ab3b40.json new file mode 100644 index 00000000000000..d3455374d18e3b --- /dev/null +++ b/change/@fluentui-react-motion-preview-20cc46cf-11ec-49a9-9f37-0e1385ab3b40.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: avoid breaking rule of hooks", + "packageName": "@fluentui/react-motion-preview", + "email": "marcosvmmoura@gmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-motion-preview/src/hooks/useMotion.test.ts b/packages/react-components/react-motion-preview/src/hooks/useMotion.test.ts index 946bf69a713e54..a554922d4f8933 100644 --- a/packages/react-components/react-motion-preview/src/hooks/useMotion.test.ts +++ b/packages/react-components/react-motion-preview/src/hooks/useMotion.test.ts @@ -1,6 +1,6 @@ import { act, renderHook } from '@testing-library/react-hooks'; -import { useMotion, MotionOptions, MotionShorthand, getDefaultMotionState, useIsMotion } from './useMotion'; +import { useMotion, MotionOptions, MotionShorthand, getDefaultMotionState } from './useMotion'; const defaultDuration = 100; const renderHookWithRef = ( @@ -89,28 +89,6 @@ describe('useMotion', () => { expect(result.current.ref).toStrictEqual(defaultState.ref); expect(result.current.active).toStrictEqual(true); }); - - it('should show error when motion changes to a different type', () => { - const spy = jest.spyOn(console, 'error').mockImplementation(() => ({})); - let defaultMotion: MotionShorthand = getDefaultMotionState(); - const { rerender } = renderHook(() => useIsMotion(defaultMotion)); - - defaultMotion = false; - - rerender(); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith( - [ - 'useMotion: The hook needs to be called with the same typeof of shorthand on every render.', - 'This is to ensure the internal state of the hook is stable and can be used to accurately detect the motion state.', - 'Please make sure to not change the shorthand on subsequent renders or to use the hook conditionally.', - '\nCurrent shorthand:', - JSON.stringify(defaultMotion, null, 2), - '\nPrevious shorthand:', - JSON.stringify(getDefaultMotionState(), null, 2), - ].join(' '), - ); - }); }); describe('when presence is false by default', () => { diff --git a/packages/react-components/react-motion-preview/src/hooks/useMotion.ts b/packages/react-components/react-motion-preview/src/hooks/useMotion.ts index 0a5db898a9b3bc..9719f387efcc10 100644 --- a/packages/react-components/react-motion-preview/src/hooks/useMotion.ts +++ b/packages/react-components/react-motion-preview/src/hooks/useMotion.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import { useAnimationFrame, useTimeout, usePrevious, useFirstMount } from '@fluentui/react-utilities'; +import { useAnimationFrame, useTimeout, useFirstMount } from '@fluentui/react-utilities'; import { useReducedMotion } from './useReducedMotion'; import { getMotionDuration } from '../utils/dom-style'; @@ -117,9 +117,7 @@ function useMotionPresence( return; } - /* - * In case animation is disabled, we can skip the animation and go straight to the idle state. - */ + // In case animation is disabled, we can skip the animation and go straight to the idle state. if (disableAnimation) { setType(presence ? 'idle' : 'unmounted'); setActive(presence); @@ -128,22 +126,16 @@ function useMotionPresence( setType(presence ? 'entering' : 'exiting'); - /* - * If the element is not rendered, nothing to do. - */ + // If the element is not rendered, nothing to do. if (!currentElement) { return; } - /* - * Wait for the next frame to ensure the element is rendered and the animation can start. - */ + // Wait for the next frame to ensure the element is rendered and the animation can start. setAnimationFrame(() => { setActive(presence); - /* - * Wait for the next frame to ensure the animation has started. - */ + // Wait for the next frame to ensure the animation has started. setAnimationFrame(() => { const duration = getMotionDuration(currentElement); @@ -165,7 +157,8 @@ function useMotionPresence( cancelAnimationFrame(); clearAnimationTimeout(); }; - /* + + /** * Only tracks dependencies that are either not stable or are used in the callbacks * This is to avoid re-running the effect on every render, especially when the element is not rendered */ @@ -207,57 +200,8 @@ export function useMotion( shorthand: MotionShorthand, options?: MotionOptions, ): MotionState { - /** - * Heads up! - * This hook returns a Motion but also accepts Motion as an argument. - * In case the hook is called with a Motion as argument, we don't need to perform the expensive computation of the - * motion state and can just return the motion value as is. This is intentional as it allows others to use the hook - * on their side without having to worry about the performance impact of the hook. - */ - // eslint-disable-next-line react-hooks/rules-of-hooks - return useIsMotion(shorthand) ? shorthand : useMotionPresence(shorthand, options); -} + const isShorthand = typeof shorthand === 'object'; + const motion = useMotionPresence(isShorthand ? false : shorthand, options); -const stringifyShorthand = (value: MotionShorthand) => { - return JSON.stringify(value, null, 2); -}; - -/** - * @internal - * - * This method emits a warning if the hook is called with - * a different typeof of shorthand on subsequent renders, - * since this can lead breaking the rules of hooks. - * - * It also return a boolean indicating whether the shorthand is a motion object. - */ -export function useIsMotion( - shorthand: MotionShorthand, -): shorthand is MotionState { - const previousShorthand = usePrevious(shorthand); - - /** - * Heads up! - * We don't want these warnings in production even though it is against native behavior - */ - if (process.env.NODE_ENV !== 'production') { - // eslint-disable-next-line react-hooks/rules-of-hooks - React.useEffect(() => { - if (previousShorthand !== null && typeof previousShorthand !== typeof shorthand) { - // eslint-disable-next-line no-console - console.error( - [ - 'useMotion: The hook needs to be called with the same typeof of shorthand on every render.', - 'This is to ensure the internal state of the hook is stable and can be used to accurately detect the motion state.', - 'Please make sure to not change the shorthand on subsequent renders or to use the hook conditionally.', - '\nCurrent shorthand:', - stringifyShorthand(shorthand), - '\nPrevious shorthand:', - stringifyShorthand(previousShorthand), - ].join(' '), - ); - } - }, [shorthand, previousShorthand]); - } - return typeof shorthand === 'object'; + return isShorthand ? shorthand : motion; }