From 98936357b799c7ee425235194c441ec7d24aa91c Mon Sep 17 00:00:00 2001 From: Keith Brown Date: Thu, 7 Jun 2018 02:26:17 -0700 Subject: [PATCH] =?UTF-8?q?Modified=20deepFreezeAndThrowOnMutationInDev=20?= =?UTF-8?q?to=20use=20Object.prototype.ha=E2=80=A6=20(#19598)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/facebook/react-native/blob/c6b96c0df789717d53ec520ad28ba0ae00db6ec2/Libraries/vendor/core/mergeInto.js#L50 https://github.com/facebook/react-native/blob/8dc3ba0444c94d9bbb66295b5af885bff9b9cd34/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 Track results of e2e tests --- scripts/run-ci-e2e-tests.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/run-ci-e2e-tests.js b/scripts/run-ci-e2e-tests.js index f3096c083a2821..589d759b7de411 100644 --- a/scripts/run-ci-e2e-tests.js +++ b/scripts/run-ci-e2e-tests.js @@ -199,11 +199,15 @@ try { exec('sleep 10s'); if (argv.tvos) { return exec( - 'xcodebuild -destination "platform=tvOS Simulator,name=Apple TV 1080p,OS=10.0" -scheme EndToEndTest-tvOS -sdk appletvsimulator test | xcpretty && exit ${PIPESTATUS[0]}', + 'xcodebuild -destination "platform=tvOS Simulator,name=Apple TV 1080p,OS=10.0" -scheme EndToEndTest-tvOS -sdk appletvsimulator test | xcpretty --report junit --output ~/react-native/reports/junit/' + + iosTestType + + '-e2e-xcodebuild-results.xml && exit ${PIPESTATUS[0]}', ).code; } else { return exec( - 'xcodebuild -destination "platform=iOS Simulator,name=iPhone 5s,OS=10.3.1" -scheme EndToEndTest -sdk iphonesimulator test | xcpretty && exit ${PIPESTATUS[0]}', + 'xcodebuild -destination "platform=iOS Simulator,name=iPhone 5s,OS=10.0" -scheme EndToEndTest -sdk iphonesimulator test | xcpretty --report junit --output ~/react-native/reports/junit/' + + iosTestType + + '-e2e-xcodebuild-results.xml && exit ${PIPESTATUS[0]}', ).code; } }, numberOfRetries)