Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve proptypes oneof warnings - Fixes #1919 #6940

Conversation

philipshurpik
Copy link

Fixes issue #1919

As I see there is two problems:

  1. If oneOf uses function in array - so they will be stringified as null - so I add functionPrettifier method as argument to JSON.stringify
  2. Possibly mismatch oneOf and oneOfType - if checks do not pass and one of the function is one of React.PropTypes - proper warning will be shown

Welcome to your suggestions

reactEurope-hackathon

@philipshurpik philipshurpik changed the title Improve proptypes oneof warnings Improve proptypes oneof warnings - Fixes #1919 Jun 1, 2016
@chicoxyzzy
Copy link
Contributor

Could you provide a test case?

for (var i = 0; i < expectedValues.length; i++) {
if (is(propValue, expectedValues[i])) {
return null;
}
if (getPropType(expectedValues[i]) === 'function' && expectedValues[i].name === 'bound checkType') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you set a flag on the bound checkType instead? This way if we change its name, this won’t start creating false positives.

@gaearon
Copy link
Collaborator

gaearon commented Jun 26, 2016

Thanks for the PR! Let’s avoid using name and also this definitely needs some tests.

@philipshurpik
Copy link
Author

thanks, will work on improvements :)

@gaearon
Copy link
Collaborator

gaearon commented Oct 23, 2016

Closing in favor of #7526 but thanks for taking the time!

@gaearon gaearon closed this Oct 23, 2016
@aweary aweary mentioned this pull request Nov 12, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants