-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
fix exception when ES6 class with no propTypes defined before calling requireNativeComponent #1260
Conversation
… requireNativeComponent
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 cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
I have a question - what kind of native view does not have custom props? All the native views so far have special props to customize them. |
@@ -24,7 +24,8 @@ function verifyPropTypes( | |||
} | |||
var nativeProps = viewConfig.nativeProps; | |||
for (var prop in nativeProps) { | |||
if (!component.propTypes[prop] && | |||
if (component.propTypes != undefined && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use ===
or !==
instead of their looser counterparts whenever possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just check truthiness without comparing to undefined
.
Can you provide more context in how you experienced this error? |
React Native "Native UI Components (iOS)" Guide call requireNativeComponent before propTypes ,Then will experience this error
|
I think there's a bug in that code, since |
@ide is right - maybe change this PR to throw a more useful error? |
Should I close the request? |
@kejinlu can you update this PR so that it asserts? |
Looks like for ES6 classes @spicyj : any idea why this is inconsistent? Is there a better mechanism than just checking both like ' |
import |
Can you figure out the right way to include the component name in the error messages and make it consistent in the function? |
@facebook-github-bot import |
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/966263836738882/int_phab to review. |
displayName || name is the right way to do it. |
ES6 classes name works, but for React.createClass displayName works.
…nance Bring over some component governance improvements to 0.68-stable
No description provided.