-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Support validation when hasOwnProperty is not in prototype #187
Conversation
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.
Tests with a null object, and an object with a hasOwnProperty property, would be great.
checkPropTypes.js
Outdated
@@ -28,7 +28,7 @@ if (process.env.NODE_ENV !== 'production') { | |||
function checkPropTypes(typeSpecs, values, location, componentName, getStack) { | |||
if (process.env.NODE_ENV !== 'production') { | |||
for (var typeSpecName in typeSpecs) { | |||
if (typeSpecs.hasOwnProperty(typeSpecName)) { | |||
if (Object.prototype.hasOwnProperty.call(typeSpecs, typeSpecName)) { |
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.
The next robustness step is to do var has = Object.prototypr.hasOwnProperty
at module scope, and then here, do has.call
.
I moved the declaration to module scope now (amended the commit) and added a test for objects with a |
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.
Thanks, LGTM!
Thanks for this fix! Running into this as well. Is there a reason it sits open? |
@mhuggins It is ready to be merged from my side (aside from the merge conflicts now). |
Rebased this; it's partially addressed by #112 but still includes tests and a fix. |
Thanks, guys! ❤️ |
Avoids the
Warning: Failed prop type: propValue.hasOwnProperty is not a function
warning whenhasOwnProperty
is not defined in the prototype chain.See #183 for details.
delete Object.prototype.hasOwnProperty
being a thing. I did not quite get this comment, so this solution might not work because of that. Just close this PR in that case.PropTypes.shape({ q: PropTypes.objectOf(PropTypes.string) })
I pointed out inobjectOf
should not requirehasOwnProperty
to be a function #183 (comment), as I didn't know where to place them