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

Fixes React 16 prop-type Errors #138

Merged
merged 1 commit into from
Feb 9, 2018
Merged

Conversation

shoerner
Copy link

When the component is started in a React 16 environment and the disabled prop is set to true, React 16 will throw a console warning for each of the functions set via the short circuit logical operators.

This currently manifests as the following (subset) of warnings logged as console errors:
image

This PR acts to silence those warnings and prep for the anticipation that React 17 will convert these to throwable errors. Documentation of the changes that introduced these warnings can be found here under "Non-boolean attributes with boolean values"

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks, this seems good. Is there a way to add a regression test for this?

@shoerner
Copy link
Author

I can give it a shot - been having a hard time getting these tests to run, especially on Windows. Will report back soon.

@shoerner
Copy link
Author

shoerner commented Feb 3, 2018

Finally got things going with a VM.

Added a test for the warning and validation of the branching logic. Let me know if it was too much or if you were looking for something more.

@elgubbo
Copy link

elgubbo commented Feb 8, 2018

Great, finally we can get rid of the annoying warnings 👍

@matfin
Copy link

matfin commented Feb 8, 2018

Excellent work on the PR. Hope this sees the light of day soon :)

@ljharb ljharb merged commit cf1ee78 into airbnb:master Feb 9, 2018
@shoerner shoerner deleted the react16Props branch June 10, 2018 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants