Skip to content

[Polymorphic] Narrow with React.ComponentType instead #648

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

Merged
merged 2 commits into from
May 12, 2021

Conversation

jjenzz
Copy link
Contributor

@jjenzz jjenzz commented May 11, 2021

Fixes #647

Replaces React.JSXElementConstructor with React.ComponentType to allow components typed by external libs in the as prop.

You can see the example from the issue working with this code here https://codesandbox.io/s/admiring-kalam-k2ltk?file=/src/App.tsx

@jjenzz jjenzz requested a review from benoitgrelard as a code owner May 11, 2021 17:54
@jjenzz jjenzz changed the title [Polymorphic] Loosen React.JSXElementConstructor type [Polymorphic] Remove React.JSXElementConstructor type May 11, 2021
@jjenzz jjenzz changed the title [Polymorphic] Remove React.JSXElementConstructor type [Polymorphic] Use React.ComponentType May 11, 2021
@jjenzz jjenzz changed the title [Polymorphic] Use React.ComponentType [Polymorphic] Narrow with React.ComponentType instead May 11, 2021
Copy link
Contributor

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks fine, obviously a 1 liner, but for my knowledge, what's the difference between JSXElementConstructor and ComponentType?

@jjenzz
Copy link
Contributor Author

jjenzz commented May 12, 2021

ComponentType is FunctionComponent | ComponentClass and they have a more complex shape with static properties on them like displayName, propTypes, defaultProps etc.

I'm guessing TS needs this additional information to be able to narrow correctly. I didn't look into it too much as I had originally wanted to use React.ComponentType anyway. I didn't previously because of an issue with inline as components but we no longer have that issue since moving everything into one overload so this works well.

@benoitgrelard
Copy link
Contributor

Ha sounds awesome then!

@benoitgrelard benoitgrelard merged commit cbfc192 into main May 12, 2021
@benoitgrelard benoitgrelard deleted the polymorphic-use-component-type branch May 12, 2021 11:56
@Kerumen
Copy link

Kerumen commented Jun 11, 2021

@jjenzz Thanks for the fix! Is it possible to release it on npm?

@benoitgrelard
Copy link
Contributor

We will be doing a release once #682 is merged.

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

Successfully merging this pull request may close these issues.

Typing for 'as' prop too strict.
3 participants