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

React Package has Breaking Change in 1.0.6 due to forwardRef #639

Closed
pantajoe opened this issue Mar 11, 2022 · 2 comments
Closed

React Package has Breaking Change in 1.0.6 due to forwardRef #639

pantajoe opened this issue Mar 11, 2022 · 2 comments

Comments

@pantajoe
Copy link

Hello there! First, thanks for this amazing icon library! ❤️

I use dependabot in my project to update the dependencies on a regular basis. Thus, without suspecting anything, I reviewed the PR that updates @heroicons/react from 1.0.5 to 1.0.6 together with its changes.

However, upgrading this dependency broke my app 😅
I relied on the types of the React package to embed heroicons and other "componentified" SVG icons into my custom Button components like this:

? `import * as React from 'react';\ndeclare function ${componentName}(props: React.ComponentProps<'svg'>): JSX.Element;\nexport default ${componentName};\n`

// props
export type BaseButtonType = 'button' | 'a';

export type BaseButtonProps<T extends BaseButtonType> = {
  as?: T;
  // ...
  icon?: (props: ComponentProps<'svg'>) => JSX.Element;
} & ComponentProps<T>;

type BaseButtonPropsWithRef<T extends BaseButtonType> = {
  innerRef?: ForwardedRef<T extends 'button' ? HTMLButtonElement : HTMLAnchorElement>;
} & BaseButtonProps<T>;

const BaseButton = <T extends BaseButtonType>({ as, children, icon, innerRef, ...props }: BaseButtonPropsWithRef<T>) => {
  return createElement(
    as ?? 'button',
    {
      ...props,
      ref: innerRef,
    },
    // the following line breaks the app
    icon?.({ className: leadingIconCssClasses, 'aria-hidden': 'true' }),
    children,
  );
};

In #614, you introduced accessing the ref on the SVGs. However, this results in those components not being callable as a function as they are now exotic components, i.e., they are now objects of the following structure:

import { PlusIcon } from '@heroicons/react/solid';

console.log(PlusIcon);
// => Object {
//   $$typeof: Symbol(react.forward_ref),
//   render: (props, ref) => { ... },
// }

And this behavior is a breaking change for me which React also suggests. However, I'm aware that this might be a niche problem and maybe not relevant for most users of this library.

I now reverted the upgrade until I come up with something that allows me to upgrade to the newest version while maintaining my api's flexibility. But it would be great if you could update the typings of this package to ForwardRefExoticComponents, so that TypeScript does not compile when relying on the previously used types of this package like me.

@RobinMalfait
Copy link
Contributor

Hey! Thank you for your bug report!
Much appreciated! 🙏

We just released v2 and I don't think we will be making changes to the v1 version anymore. However, I'm curious what you mean by:

However, this results in those components not being callable as a function as they are now exotic components

A "functional" component like:

function MyComponent() {
  return <div />
}

Should never be called like MyComponent(), but should always be used with JSX like <MyComponent /> or React.createElement(MyComponent).

Were you calling the icons as actual functions like ArchiveIcon()?

@pantajoe
Copy link
Author

Hi! Thanks for your answer.
You are completely right with your observation. When I submitted this issue, I was quite new to React 😅

Sorry for the rather dumb example. I wanted to assign the heroicons as a prop to buttons, i.e.,

<Button {...props} icon={ArchiveIcon} />

And I thought wrapping the entire button in createElement and calling the icon as a function from within the Button would be fine 😅

Simply using JSX notation solved everything. Also forgot to update and close this issue.

Thanks and sorry again for this "bug report"!

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

No branches or pull requests

2 participants