Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Element Types JSX.IntrinsicElements vs React.[Element]HTMLAttributes* #194

Open
micotodev opened this issue Apr 21, 2023 · 1 comment
Open
Labels

Comments

@micotodev
Copy link

Thank you for all the work you have done on this library, it's helped tremendously.

Not a bug report but just a question around typings in this library.

I am creating my own select/button components etc and usually use React.[Element]HTMLAttributes<HTML[Element]Element types instead of using JSX.InstrinsicElements["element"], which this library uses. Main issue is that JSX.IntrinsicElements always passes a ref as a prop which has the LegacyRef type rather than Ref, and always passes a ref even when it will not actually work.

Example, my button component is as below (removed imports and variants for brevity):

const CustomButton = React.forwardRef<HTMLButtonElement, ButtonProps>(
  ({ className, variant, size, ...props }, ref) => {
    return (
      <button
        className={clsx(buttonVariants({ variant, size, className }))}
        ref={ref}
        {...props}
      />
    )
  }
)

buttonComponent import from the above:

const Button = (props: JSX.IntrinsicElements['button']) => (
  <CustomButton
    className="w-full justify-center"
    variant="default"
    {...props}
  />
)

In this scenario, the incompatible LegacyRef is forwarded through as a prop from JSX.IntrinsicElements

TS Feedback:

Type 'LegacyRef<HTMLButtonElement> | undefined' is not assignable to type 'Ref<HTMLButtonElement> | undefined'.
      Type 'string' is not assignable to type 'Ref<HTMLButtonElement> | undefined'.

Forgive my naivety if this is a specific design decision, this article goes a little more in depth into the issues I am running into.

@danielweinmann
Copy link
Contributor

Hey, @micotodev! Thanks for the issue and the article. I wasn't aware of this difference. I believe we can be more explicit about our types with or without ref in v2.

We also have #121, where we'll try to make the whole component props generic in v2. But I'll leave this issue open in case we can't make them generic enough and want to be more explicit.

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants