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.

https://github.com/marcosmoura/fluentui/blob/17f04063d4360843bf44dde93ccb7abc34826053/rfcs/react-components/convergence/component-transitions-on-mount-or-unmount.md#api

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} />
}

Example

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/A

Requested 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.