facebook
checkPropTypes: add argument that allows external logging
When using prop-types for checking things outside of React, having checkPropTypes throw an error instead of swallowing it and logging it to console.error can be useful. The flag throwErrors will throw the error if set to true instead of logging it. AcheckPropTypeWithErrors convenience function, which calls checkPropTypes with throwErrors set to true has also been added.
Fixes #34
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!
If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions.
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@gaearon Is this a good approach to using prop-types checking outside of React? Or is there a better way?
Sorry, we're busy with many other things right now, and it's unlikely we'll have time to review PRs to prop-types very soon. In this PR, a few things concern me:
- Creating errors is expensive and we'd like to get away from that, but
throwing reinforces this pattern - This doesn't expose other important information (such as component stack)
The PR has been updated to not throw an error, but instead calls the function passed in as an argument, with the same arguments as are passed to warning.
I'm aware that there might not be time to review this PR, but with React 16 in beta now the resolution on how to use prop-types with custom errors is becoming a more prominent issue.
I was wondering if there is anything else that would need to be done on this PR to have it merged. Or if such a feature won't be considered for the prop-types package, we'll look into alternatives.
@kaiyoma I used check-prop-types module mentioned here: https://github.com/facebook/prop-types/issues/28#issuecomment-313504898
This would also be super useful to me. Any word on whether this might make it in? If it won't, then closing this PR and "officially" referring users to another project that does this might be a good move.
oh my gosh so close, would love to see this added to the library, for sure.
This has been here for quite a while, and have read little to no objections to merging this PR aside from "time" constraints. This would be a wonderful addition to the propTypes library and I am genuinely confused as to what it would take to consider merging it.