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

Add validation for arrayOf and objectOf in ReactPropTypes #5390

Merged
merged 2 commits into from Nov 12, 2015

Conversation

Projects
None yet
4 participants
@chicoxyzzy
Copy link
Contributor

commented Nov 4, 2015

  static propTypes = {
    instrument: PropTypes.object.isRequired,
    data: PropTypes.arrayOf({
      timestamp: PropTypes.object.isRequired,
      value: PropTypes.number.isRequired,
    }).isRequired,
    height: PropTypes.number.isRequired,
    width: PropTypes.number.isRequired,
  };

Notice data property. it should look like

    data: PropTypes.arrayOf(PropTypes.shape({
      timestamp: PropTypes.object.isRequired,
      value: PropTypes.number.isRequired,
    })).isRequired,

instead.

This is one of common mistakes. But error message is very cryptic:
Warning: Failed propType: typeChecker is not a function Check the render method of 'ChartPanel'.

So I wrote some tests and pretty error messages.

now error looks like this
Warning: Failed propType: Invalid prop 'data' supplied to 'ChartContainer', expected valid PropType. Check the render method of 'ChartPanel'.

This is kind of same as #3963 but for arrayOf and objectOf proptypes

@chicoxyzzy chicoxyzzy force-pushed the chicoxyzzy:arrayof-objectof-tests branch from a180f60 to 7ae6791 Nov 4, 2015

@chicoxyzzy chicoxyzzy changed the title Adding validation for arrayOf and objectOf in ReactPropTypes Add validation for arrayOf and objectOf in ReactPropTypes Nov 4, 2015

@@ -144,6 +144,11 @@ function createAnyTypeChecker() {

function createArrayOfTypeChecker(typeChecker) {
function validate(props, propName, componentName, location, propFullName) {
if (typeof typeChecker !== 'function') {
return new Error(
`Invalid argument \`${propFullName}\` supplied to \`${componentName}\`, expected valid PropType.`

This comment has been minimized.

Copy link
@zpao

zpao Nov 5, 2015

Member

This message isn't awesome. You're actually trying to say that the component author wrote a bad propType spec, but the way this error will show sounds like the user provided an invalid prop value. Can we tune it so that's more clear?

This comment has been minimized.

Copy link
@chicoxyzzy

chicoxyzzy Nov 5, 2015

Author Contributor

Is it more clear now?

@zpao

This comment has been minimized.

Copy link
Member

commented Nov 5, 2015

Thanks for doing this, I think it's a great idea. I sort of wish I had pushed for making #3963 actually throw an error (since it would throw at require time, not component creation time). We have less contextual information we can provide but the error would actually be thrown earlier, preventing bad proptype definitions from getting committed… maybe we can do that in another pass.

@chicoxyzzy chicoxyzzy force-pushed the chicoxyzzy:arrayof-objectof-tests branch from 02d14b3 to a86d25d Nov 5, 2015

@facebook-github-bot

This comment has been minimized.

Copy link

commented Nov 5, 2015

@chicoxyzzy updated the pull request.

@zpao

This comment has been minimized.

Copy link
Member

commented Nov 12, 2015

Alright, let's do it. Now that I'm thinking about proptypes again, I might have some followup ideas to try but this puts us in a better spot.

zpao added a commit that referenced this pull request Nov 12, 2015

Merge pull request #5390 from chicoxyzzy/arrayof-objectof-tests
Add validation for arrayOf and objectOf in ReactPropTypes

@zpao zpao merged commit 8104262 into facebook:master Nov 12, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chicoxyzzy chicoxyzzy deleted the chicoxyzzy:arrayof-objectof-tests branch Nov 12, 2015

@slorber

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2015

hey @zpao I also got a case where I had this error. I'm not exactly sure where it came from nor where I fixed it but a better error message would have helped :)

Actually we register all of our Api payloads as proptypes. I think at some point the declaration order of proptypes lead to something like PT.arrayOf(undefined), or maybe PT.shape({attribute: undefined}

Maybe it's something else because I can't get this "typeChecker" error back.

@chicoxyzzy

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2015

@slorber feel free to ping me if you'll meet your issue again

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

Update dependency react to v0.14.9 #29

0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.