Modified deepFreezeAndThrowOnMutationInDev to use Object.prototype.ha… (#19598)
Summary: This PR fixes a bug in `deepFreezeAndThrowOnMutationInDev` which did not take into account that objects passed to it may have been created with `Object.create(null)` and thus may not have a prototype. Such objects don't have the methods `hasOwnProperty`, `__defineGetter__`, or `__defineSetter__` on the instance. I ran into an unrecoverable error in React Native when passing this type of object across the bridge because `deepFreezeAndThrowOnMutationInDev` attempts to call `object.hasOwnProperty(key)`, `object.__defineGetter__` and `object__defineSetter__` on objects passed to it. But my object instance does not have these prototype methods. Changes: * Defined `Object.prototype.hasOwnProperty` as a `const` (pattern used elsewhere in React Native) * Modified calls to `object.hasOwnProperty(key)` to use `hasOwnProperty.call(object, key)` (Per ESLint rule [here](https://eslint.org/docs/rules/no-prototype-builtins)) * Modified calls to deprecated methods `object.__defineGetter__` and `object.__defineSetter__` to instead use `Object.defineProperty` to define get and set methods on the object. (Per guidance on [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/__defineGetter__)) * Added a new test to `deepFreezeAndThrowOnMutationInDev-test` to verify the fix. I tried to create a reproducible example to post to Snack by passing prototype-less objects to a `Text` component, in various ways, but they appear to be converted to plain objects before crossing the bridge and therefore they do not throw an error. However, I was able to create a new test to reproduce the issue. I added the following test to `deepFreezeAndThrowOnMutationInDev-test`: ```JavaScript it('should not throw on object without prototype', () => { __DEV__ = true; var o = Object.create(null); o.key = 'Value'; expect(() => deepFreezeAndThrowOnMutationInDev(o)).not.toThrow(); }); ``` The changes in this PR include this new test. ESLint test produced no change in Error count (3) or Warnings (671) N/A Other areas with _possibly_ the same issue:c6b96c0df7/Libraries/vendor/core/mergeInto.js (L50)
8dc3ba0444/Libraries/ReactNative/requireNativeComponent.js (L134)
[GENERAL] [BUGFIX] [Libraries/Utilities/deepFreezeAndThrowOnMutationInDev] -Fix for compatibility with objects without a prototype. Closes https://github.com/facebook/react-native/pull/19598 Differential Revision: D8310845 Pulled By: TheSavior fbshipit-source-id: 020c414a1062a637e97f9ee99bf8e5ba2d1fcf4f
This commit is contained in:
parent
cb1bdf1e37
commit
02d1bcc047
|
@ -26,6 +26,13 @@ describe('deepFreezeAndThrowOnMutationInDev', function() {
|
||||||
expect(() => deepFreezeAndThrowOnMutationInDev()).not.toThrow();
|
expect(() => deepFreezeAndThrowOnMutationInDev()).not.toThrow();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not throw on object without prototype', () => {
|
||||||
|
__DEV__ = true;
|
||||||
|
var o = Object.create(null);
|
||||||
|
o.key = 'Value';
|
||||||
|
expect(() => deepFreezeAndThrowOnMutationInDev(o)).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
it('should throw on mutation in dev with strict', () => {
|
it('should throw on mutation in dev with strict', () => {
|
||||||
'use strict';
|
'use strict';
|
||||||
__DEV__ = true;
|
__DEV__ = true;
|
||||||
|
|
|
@ -39,12 +39,17 @@ function deepFreezeAndThrowOnMutationInDev<T: Object>(object: T): T {
|
||||||
}
|
}
|
||||||
|
|
||||||
const keys = Object.keys(object);
|
const keys = Object.keys(object);
|
||||||
|
const hasOwnProperty = Object.prototype.hasOwnProperty;
|
||||||
|
|
||||||
for (var i = 0; i < keys.length; i++) {
|
for (var i = 0; i < keys.length; i++) {
|
||||||
var key = keys[i];
|
var key = keys[i];
|
||||||
if (object.hasOwnProperty(key)) {
|
if (hasOwnProperty.call(object, key)) {
|
||||||
object.__defineGetter__(key, identity.bind(null, object[key]));
|
Object.defineProperty(object, key, {
|
||||||
object.__defineSetter__(key, throwOnImmutableMutation.bind(null, key));
|
get: identity.bind(null, object[key]),
|
||||||
|
});
|
||||||
|
Object.defineProperty(object, key, {
|
||||||
|
set: throwOnImmutableMutation.bind(null, key),
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -53,7 +58,7 @@ function deepFreezeAndThrowOnMutationInDev<T: Object>(object: T): T {
|
||||||
|
|
||||||
for (var i = 0; i < keys.length; i++) {
|
for (var i = 0; i < keys.length; i++) {
|
||||||
var key = keys[i];
|
var key = keys[i];
|
||||||
if (object.hasOwnProperty(key)) {
|
if (hasOwnProperty.call(object, key)) {
|
||||||
deepFreezeAndThrowOnMutationInDev(object[key]);
|
deepFreezeAndThrowOnMutationInDev(object[key]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue