adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.76k stars 1.09k forks source link

react-aria button hooks/framer sample does not build #5713

Open rolanday opened 8 months ago

rolanday commented 8 months ago

Provide a general summary of the issue here

The Hooks sample provided at the end of https://react-spectrum.adobe.com/react-aria/Button.html results in the following typescript error , when built as-is (i.e., using the code sample provided):

app/components/core/button.tsx:22:9 - error TS2322: Type 'ReactNode | ((values: ButtonRenderProps) => ReactNode)' is not assignable to type 'ReactNode | MotionValue<number> | MotionValue<string>'.
  Type '(values: ButtonRenderProps) => ReactNode' is not assignable to type 'ReactNode | MotionValue<number> | MotionValue<string>'.

22         {props.children}
           ~~~~~~~~~~~~~~~~

  node_modules/framer-motion/dist/index.d.ts:3177:5
    3177     children?: React.ReactNode | MotionValue<number> | MotionValue<string>;
             ~~~~~~~~
    The expected type comes from property 'children' which is declared here on type 'IntrinsicAttributes & HTMLAttributesWithoutMotionProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement> & MotionProps & RefAttributes<...>'

Issues seems to be related to https://github.com/framer/motion/issues/1723 , but not 100% sure and code-sample should build so filing separately.

๐Ÿค” Expected Behavior?

Code sample provided successuflly builds.

๐Ÿ˜ฏ Current Behavior

import type {ButtonProps} from 'react-aria-components';
import {ButtonContext, useContextProps} from 'react-aria-components';
import {useButton} from 'react-aria';
import {motion} from 'framer-motion';

const AnimatedButton = React.forwardRef(
  (props: ButtonProps, ref: React.ForwardedRef<HTMLButtonElement>) => {
    // Merge the local props and ref with the ones provided via context.
    [props, ref] = useContextProps(props, ref, ButtonContext);

    let { buttonProps, isPressed } = useButton(props, ref);
    return (
      <motion.button
        {...buttonProps}
        ref={ref}
        animate={{
          scale: isPressed ? 0.9 : 1
        }}
      >
        {props.children} // <== Typescript error
      </motion.button>
    );
  }
);

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

Install react-aria-components 1.0.1 to TS configured project and attempt to build, then note error.

Version

react-aria-components 1.0.1

What browsers are you seeing the problem on?

Firefox, Chrome, Safari

If other, please specify.

No response

What operating system are you using?

macOS

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

reidbarber commented 8 months ago

Could you share a codesandbox/stackblitz that is showing this error (or include the tsconfig)? I wasn't able to reproduce this here: https://stackblitz.com/edit/stackblitz-starters-brhygm

rolanday commented 8 months ago

Hi Reid, thanks for taking a look. Here's a sandbox link that repros the TS error. (Note, functionally, the code works as expected). The linked project was created w/ vanilla remix config, + react-aria-components and framer-motion deps added. Please see <project-root>\app\components\button.tsx , line 21. It's the exact same error I get on my devbox (currently @ts-ignore'ing it).

https://codesandbox.io/p/devbox/react-aria-framer-button-rhc83m?file=%2Fapp%2Fcomponents%2Fbutton.tsx

Thanks again!

snowystinger commented 8 months ago

Seems to stem from us allowing a function as children for render props. So you could change the types that your button accepts to remove that. Omit<ButtonProps, "children"> & {children: React.ReactNode}

or you could check for a function and call it, or make use of composeRenderProps if you wanted to keep it as an option.

rolanday commented 8 months ago

H @snowystinger , thanks for the suggestions! No luck on fist -- fixes one type error and results in another (on <motion.button el). Same link edited to show: https://codesandbox.io/p/devbox/react-aria-framer-button-rhc83m?file=%2Fapp%2Fcomponents%2Fbutton.tsx

snowystinger commented 8 months ago

Probably could omit onAnimationStart as well @devongovett I feel like I saw somewhere that you used our hooks with motion, is this what you did?