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

checkPropTypes() suppresses repeat warning messages about missing required propTypes #6293

Closed
GordyD opened this issue Mar 18, 2016 · 1 comment

Comments

@GordyD
Copy link
Contributor

GordyD commented Mar 18, 2016

We have been writing some tests around behaviour when required prop types are not supplied to a React component. We noticed that as we added .isRequired to more PropTypes some of our test suite did not break as expected. Our first tests check the number of times console.warn is called, which failed as this number would increase, but the following tests, that did not supply the newly required prop, did not trigger an increase.

To demonstrate the behaviour in a browser please see this jsFiddle.

We discovered the cause in ReactElementValidator. It is actually suppressing repeat warning messages for missing propTypes. As you can see in the source, there is even a comment about this issue.

Only monitor this failure once because there tends to be a lot of the same error.

From our perspective, this non-deterministic behaviour is unexpected and we would expect these warnings to appear any time we render a component without props which we have specified as being required, even if there tends to be a lot of them! We've discussed it as a team and we feel that the greater community may expect this type of behaviour too.

At this point we'd appreciate it if we could either:

  • get perspective on why this behaviour exists (we may be missing something that this protects against)
  • find a solution to patch this behaviour (remove suppression would be optimal in our eyes)

The relevant commit when this behaviour was added is here, quite a while ago now!

GordyD added a commit to tidepool-org/blip that referenced this issue Mar 18, 2016
After wrestling with an issue inside React that suppresses repeat missing propType warnings see facebook/react#6293 Jana and I have concluded that we can test the propType validation effectively as long as we first test the rendering of the component with all props specified and then test for warnings when no props specified, second.
@gaearon
Copy link
Collaborator

gaearon commented Mar 18, 2016

I think this is the same as #4302 so I will close in its favor. This is definitely something we plan to improve for testing purposes, and gradually moving our warnings to the new devtools API in #5306 should make this possible.

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

No branches or pull requests

2 participants