From d894532700c7364ae385efe8765fdf6cc0b0734f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Dec 2017 18:27:17 +0000 Subject: [PATCH 1/3] Add a test for bad Map polyfill --- .../src/__tests__/ReactIncremental-test.js | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js index ce98b306cdd59..72ba93fc098b8 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js @@ -2715,4 +2715,77 @@ describe('ReactIncremental', () => { expect(ReactNoop.flush()).toEqual([]); }); + + it('does not break with a bad Map polyfill', () => { + const realMapSet = Map.prototype.set; + + function triggerCodePathThatUsesFibersAsMapKeys() { + function Thing() { + throw new Error('No.'); + } + class Boundary extends React.Component { + state = {didError: false}; + componentDidCatch() { + this.setState({didError: true}); + } + render() { + return this.state.didError ? null : ; + } + } + ReactNoop.render(); + ReactNoop.flush(); + } + + // First, verify that this code path normally receives Fibers as keys, + // and that they're not extensible. + jest.resetModules(); + let receivedNonExtensibleObjects; + // eslint-disable-next-line no-extend-native + Map.prototype.set = function(key) { + if (typeof key === 'object' && key !== null) { + if (!Object.isExtensible(key)) { + receivedNonExtensibleObjects = true; + } + } + return realMapSet.apply(this, arguments); + }; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + try { + receivedNonExtensibleObjects = false; + triggerCodePathThatUsesFibersAsMapKeys(); + } finally { + // eslint-disable-next-line no-extend-native + Map.prototype.set = realMapSet; + } + // If this fails, find another code path in Fiber + // that passes Fibers as keys to Maps. + // Note that we only expect them to be non-extensible + // in development. + expect(receivedNonExtensibleObjects).toBe(__DEV__); + + // Next, verify that a Map polyfill that "writes" to keys + // doesn't cause a failure. + jest.resetModules(); + // eslint-disable-next-line no-extend-native + Map.prototype.set = function(key, value) { + if (typeof key === 'object' && key !== null) { + // A polyfill could do something like this. + // It would throw if an object is not extensible. + key.__internalValueSlot = value; + } + return realMapSet.apply(this, arguments); + }; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + try { + triggerCodePathThatUsesFibersAsMapKeys(); + } finally { + // eslint-disable-next-line no-extend-native + Map.prototype.set = realMapSet; + } + // If we got this far, our feature detection worked. + // We knew that Map#set() throws for non-extensible objects, + // so we didn't set them as non-extensible for that reason. + }); }); From da4e2113f00d62c6c4e092b63dd57f5ef3329322 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Dec 2017 18:34:38 +0000 Subject: [PATCH 2/3] Add a workaround for the Rollup bug --- packages/react-reconciler/src/ReactFiber.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 941247574f58a..7b69db366948e 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -41,10 +41,12 @@ if (__DEV__) { var hasBadMapPolyfill = false; try { var nonExtensibleObject = Object.preventExtensions({}); - /* eslint-disable no-new */ - new Map([[nonExtensibleObject, null]]); - new Set([nonExtensibleObject]); - /* eslint-enable no-new */ + var testMap = new Map([[nonExtensibleObject, null]]); + var testSet = new Set([nonExtensibleObject]); + // This is necessary for Rollup to not consider these unused. + // TODO: report this as a Rollup bug. + testMap.set(0, 0); + testSet.add(0); } catch (e) { // TODO: Consider warning about bad polyfills hasBadMapPolyfill = true; From cd30dc6ef515729445f2ca0ba83b54809ec7e302 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Dec 2017 18:59:10 +0000 Subject: [PATCH 3/3] Add a link to the bug URL --- packages/react-reconciler/src/ReactFiber.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 7b69db366948e..257dcd65a2442 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -44,7 +44,8 @@ if (__DEV__) { var testMap = new Map([[nonExtensibleObject, null]]); var testSet = new Set([nonExtensibleObject]); // This is necessary for Rollup to not consider these unused. - // TODO: report this as a Rollup bug. + // https://github.com/rollup/rollup/issues/1771 + // TODO: we can remove these if Rollup fixes the bug. testMap.set(0, 0); testSet.add(0); } catch (e) {