Skip to content

Add support for Flow IntersectionTypeAnnotation to prop-types and no-unused-prop-types #1404

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 7 commits into from
Aug 30, 2017

Conversation

jseminck
Copy link
Contributor

@jseminck jseminck commented Aug 29, 2017

Fixes #1364. Fixes #1323.

The following snippet creates an IntersectionTypeAnnotation which was not handled in both rules.

type Props = PropsA & PropsB;

I hope the tests/implementation are speaking for themselves on this one 😄 If anyone has any advice for additional test cases please let me know. I didn't test the React.Component<Props> syntax because I figured these shouldn't affect that implementation, but perhaps it's worth it?

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This seems good. Are there any other rules that would benefit from this awareness?

@jseminck
Copy link
Contributor Author

Hmm - yes, I think that default-props-match-prop-types doesn't support it either. Will check.

@jseminck
Copy link
Contributor Author

Added support to default-props-match-prop-types as well. Enjoy!

This one was a bit harder because the code is structured completely differently from the other 2 rules, even though the props detection should handle all the same cases... hopefully I can fix that issue soon. 😄

props: Props;
static defaultProps = {
foo: "foo",
bar: "bar",
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test case where a defaultProp is omitted for a ? prop type, and there's no error? Alternatively, you could just remove the defaultProp for "bar" here.

@LINKIWI
Copy link

LINKIWI commented Aug 30, 2017

Can confirm that this resolves #1364. Thanks for your work!

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

Successfully merging this pull request may close these issues.

3 participants