-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Chore: Add tests for prop-types destructuring #2029
Conversation
@sstern6 Would it be possible to add a deep-destructuring example as well? For example: class MyComponent extends React.PureComponent {
propTypes = {
foo: PropTypes.shape({
// TODO: fill this out
}).isRequired,
}
render() {
const { foo: { bar } } = this.props;
return <div>{bar}</div>;
}
} Expected error would be: I ask for this because I've seen cases where sometimes destructuring works on the outer layer but not when you go deeper. |
@MatthewHerbst just checked locally and looks like with the example:
Does not throw any errors. This PR was to test for spread destructruing and the errors associated with that. I can open a new ticket that will include the test you asked for and the fix, how does that sound? How do we feel about this? |
@sstern6 #1422 actually reports the problem noted above, which is why I was hoping to see a deep-destructuring test in the test suite that purportedly tested it heh. Seems #1422 still isn't fixed though. I think the tests you have above are good to add regardless, but I don't think they should be said to fully fix and/or verify #1422 |
Youre right sorry about that, ill get on a fix for the deep destructuring, got caught up in the spread operator examples. |
Amazing, thank you! If there's any way I can help, please let me know. I've taken a glance at it before and didn't really know where to start, but happy to look again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but it seems like i should wait for a few additional tests to be added.
Fixes and verifies: #1422