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

Use of ComponentProps should possibly not be recommended #170

Open
s-h-a-d-o-w opened this issue May 15, 2019 · 1 comment
Open

Use of ComponentProps should possibly not be recommended #170

s-h-a-d-o-w opened this issue May 15, 2019 · 1 comment

Comments

@s-h-a-d-o-w
Copy link
Contributor

Reason being that it doesn't work properly with class components that have default props:

class Foo extends React.Component<{bar: number}> {
  static defaultProps = {
    bar: 123,
  };
}

const props: React.ComponentProps<typeof Foo> = {};
const foo = <Foo />; // Of course works

Results in: Property 'bar' is missing in type '{}' but required in type '{ bar: number; }'.ts(2741)

Also, the team who originally wrote it said it was only supposed to be used internally.

Considering that that discussion about improving ComponentProps I just linked to ended without getting resolved, I'd like to suggest to go with one of the following for this guide:

  • Instead of ComponentProps, have a code block with a custom type for people to use - possibly the one from the PR (since it does work). (While I personally do this in my projects, it seems to me that you're trying to stick with what's possible out of the box, so this might not be an acceptable solution for this guide, I suppose.)
  • Completely remove the part about ComponentProps from the README.
  • Warn about the shortcomings of using the vanilla ComponentProps.

I'd also like to note that there's a second shortcoming to both ComponentProps as well as my enhanced version: It's just not possible to use this with generic class components. Since TS doesn't support something like React.ComponentProps<typeof Foo<string>>. As far as I know. But one of course can export generic prop types.

@piotrwitek
Copy link
Owner

Hey @s-h-a-d-o-w , thanks a lot for sharing your findings with us 👍

For now, I'll add a warning to ComponentProps type in the guide until I can find out some alternative solution.

I haven't studied the linked thread yet.

piotrwitek added a commit that referenced this issue May 17, 2019
- Added new react type example HTMLProps
- Added note about ComponentProps shortcomings
Related #170
piotrwitek added a commit that referenced this issue May 17, 2019
…ed note about ComponentProps shortcomingsRelated #170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants