-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Description
Library
React Components / v9 (@fluentui/react-components)
System Info
N/A, latest `master`Are you reporting Accessibility issue?
no
Reproduction
N/A
Bug Description
According to the RFC:
The hook always returns a MotionState. The received MotionShorthand parameter can be either a boolean or a MotionState. This flexibility is extremely useful for cases when an override on another component is needed, and with that a double calculation is avoided.
The usage examples are there:
- https://github.com/marcosmoura/fluentui/blob/17f04063d4360843bf44dde93ccb7abc34826053/rfcs/react-components/convergence/component-transitions-on-mount-or-unmount.md#application-side
- https://react.fluentui.dev/?path=/docs/preview-components-drawer--default#motion-custom
Note: When
useMotion()gets a longhand i.e.useMotion(useMotion())it returns an object that have been passed without any modifications (#example). The whole logic in the hook is skipped then, so it's more like a controlled motion.
The described use case makes sense, however it violates rules of hooks - hooks can't be called conditionally.
Why it may be not a big problem for Drawer, but for Dialog (#29391) it's a bigger concern due the high amount of existing usages. By design, there is nothing that forces users to avoid conditions i.e. the following scenario:
function App() {
const [open, setOpen] = React.useState()
const [animated, setAnimated] = React.useState()
const motion = useMotion(open)
return <Drawer motion={animated ? motion : open} />
}That's especially confusing for consumers that are familiar with shorthands & slots as they don't have such limitations.
Note, useMotion() should emit an error in this scenario, but it does not (example):
Uncaught Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement.
Uncaught Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement.
The above error occurred in the <Component> component:
Uncaught Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement.
Anyway, validation in runtime does not seem sufficient for this case.
Actual Behavior
Uncaught Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement.
useMotion() violates rules of hooks.
Expected Behavior
useMotion() does not violate rules of hooks.
Logs
N/ARequested priority
High
Products/sites affected
N/A
Are you willing to submit a PR to fix?
no
Validations
- Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- The provided reproduction is a minimal reproducible example of the bug.