From 15ae5857f6a86ead4143dc100c61d27736cf29d9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 16 Jul 2016 20:51:25 +0100 Subject: [PATCH] Eagerly evaluate inline requires in Jest (#7245) * Eagerly evaluate inline requires in Jest I inlined some requires in #7188 to fix the build size regression. However this caused an issue with Jest due to it resetting module registry between tests. This is a temporary fix to #7240. It should be reverted as part of #7178. * Make the hack work in all environments --- grunt/tasks/browserify.js | 10 ++++++++++ .../classic/types/checkReactTypeSpec.js | 19 ++++++++++++++++++- .../stack/reconciler/ReactChildReconciler.js | 19 ++++++++++++++++++- .../ReactMultiChildReconcile-test.js | 6 ------ .../stack/reconciler/__tests__/refs-test.js | 8 -------- src/shared/utils/flattenChildren.js | 19 ++++++++++++++++++- 6 files changed, 64 insertions(+), 17 deletions(-) diff --git a/grunt/tasks/browserify.js b/grunt/tasks/browserify.js index 3c9b2c089bf5..50a34d6f687f 100644 --- a/grunt/tasks/browserify.js +++ b/grunt/tasks/browserify.js @@ -25,6 +25,16 @@ module.exports = function() { entries: entries, debug: config.debug, // sourcemaps standalone: config.standalone, // global + insertGlobalVars: { + // We can remove this when we remove the few direct + // process.env.NODE_ENV checks against "test". + // The intention is to avoid embedding Browserify's `process` shim + // because we don't really need it. + // See https://github.com/facebook/react/pull/7245 for context. + process: function() { + return 'undefined'; + }, + }, }; var bundle = browserify(options); diff --git a/src/isomorphic/classic/types/checkReactTypeSpec.js b/src/isomorphic/classic/types/checkReactTypeSpec.js index 189a3d0a695b..84211727fa97 100644 --- a/src/isomorphic/classic/types/checkReactTypeSpec.js +++ b/src/isomorphic/classic/types/checkReactTypeSpec.js @@ -17,6 +17,21 @@ var ReactPropTypesSecret = require('ReactPropTypesSecret'); var invariant = require('invariant'); var warning = require('warning'); +var ReactComponentTreeDevtool; + +if ( + typeof process !== 'undefined' && + process.env && + process.env.NODE_ENV === 'test' +) { + // Temporary hack. + // Inline requires don't work well with Jest: + // https://github.com/facebook/react/issues/7240 + // Remove the inline requires when we don't need them anymore: + // https://github.com/facebook/react/pull/7178 + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool') +} + var loggedTypeFailures = {}; /** @@ -73,7 +88,9 @@ function checkReactTypeSpec(typeSpecs, values, location, componentName, element, var componentStackInfo = ''; if (__DEV__) { - var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + if (!ReactComponentTreeDevtool) { + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + } if (debugID !== null) { componentStackInfo = ReactComponentTreeDevtool.getStackAddendumByID(debugID); } else if (element !== null) { diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js index 40f589c43523..0d94b766d8de 100644 --- a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js @@ -19,11 +19,28 @@ var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); +var ReactComponentTreeDevtool; + +if ( + typeof process !== 'undefined' && + process.env && + process.env.NODE_ENV === 'test' +) { + // Temporary hack. + // Inline requires don't work well with Jest: + // https://github.com/facebook/react/issues/7240 + // Remove the inline requires when we don't need them anymore: + // https://github.com/facebook/react/pull/7178 + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool') +} + function instantiateChild(childInstances, child, name, selfDebugID) { // We found a component instance. var keyUnique = (childInstances[name] === undefined); if (__DEV__) { - var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + if (!ReactComponentTreeDevtool) { + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + } warning( keyUnique, 'flattenChildren(...): Encountered two children with the same key, ' + diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js index 50a1a01e208f..034d839b0fc0 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js @@ -278,12 +278,6 @@ function testPropsSequence(sequence) { describe('ReactMultiChildReconcile', function() { beforeEach(function() { jest.resetModuleRegistry(); - - React = require('React'); - ReactDOM = require('ReactDOM'); - ReactDOMComponentTree = require('ReactDOMComponentTree'); - ReactInstanceMap = require('ReactInstanceMap'); - mapObject = require('mapObject'); }); it('should reset internal state if removed then readded', function() { diff --git a/src/renderers/shared/stack/reconciler/__tests__/refs-test.js b/src/renderers/shared/stack/reconciler/__tests__/refs-test.js index db7f5ad57ef5..86010932b2c1 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/refs-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/refs-test.js @@ -116,10 +116,6 @@ var expectClickLogsLengthToBe = function(instance, length) { describe('reactiverefs', function() { beforeEach(function() { jest.resetModuleRegistry(); - - React = require('React'); - ReactTestUtils = require('ReactTestUtils'); - reactComponentExpect = require('reactComponentExpect'); }); /** @@ -163,10 +159,6 @@ describe('reactiverefs', function() { describe('ref swapping', function() { beforeEach(function() { jest.resetModuleRegistry(); - - React = require('React'); - ReactTestUtils = require('ReactTestUtils'); - reactComponentExpect = require('reactComponentExpect'); }); var RefHopsAround = React.createClass({ diff --git a/src/shared/utils/flattenChildren.js b/src/shared/utils/flattenChildren.js index 3e21cc550315..e0e1ed870959 100644 --- a/src/shared/utils/flattenChildren.js +++ b/src/shared/utils/flattenChildren.js @@ -16,6 +16,21 @@ var KeyEscapeUtils = require('KeyEscapeUtils'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); +var ReactComponentTreeDevtool; + +if ( + typeof process !== 'undefined' && + process.env && + process.env.NODE_ENV === 'test' +) { + // Temporary hack. + // Inline requires don't work well with Jest: + // https://github.com/facebook/react/issues/7240 + // Remove the inline requires when we don't need them anymore: + // https://github.com/facebook/react/pull/7178 + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool') +} + /** * @param {function} traverseContext Context passed through traversal. * @param {?ReactComponent} child React child component. @@ -33,7 +48,9 @@ function flattenSingleChildIntoContext( const result = traverseContext; const keyUnique = (result[name] === undefined); if (__DEV__) { - var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + if (!ReactComponentTreeDevtool) { + ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); + } warning( keyUnique, 'flattenChildren(...): Encountered two children with the same key, ' +