From 4be65283e50eaef44d0c4126a24beccfd1d746d4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 4 Apr 2018 16:07:32 +0100 Subject: [PATCH] Add a build step to hoist warning conditions --- .../wrap-warning-with-env-check-test.js | 2 +- .../wrap-warning-with-env-check.js | 22 ++++++++++++++----- scripts/jest/preprocessor.js | 6 +++++ scripts/rollup/build.js | 4 ++-- 4 files changed, 26 insertions(+), 8 deletions(-) rename scripts/{rollup/plugins => babel}/__tests__/wrap-warning-with-env-check-test.js (92%) rename scripts/{rollup/plugins => babel}/wrap-warning-with-env-check.js (63%) diff --git a/scripts/rollup/plugins/__tests__/wrap-warning-with-env-check-test.js b/scripts/babel/__tests__/wrap-warning-with-env-check-test.js similarity index 92% rename from scripts/rollup/plugins/__tests__/wrap-warning-with-env-check-test.js rename to scripts/babel/__tests__/wrap-warning-with-env-check-test.js index 3836a1e6e4dcc..7e81a663428aa 100644 --- a/scripts/rollup/plugins/__tests__/wrap-warning-with-env-check-test.js +++ b/scripts/babel/__tests__/wrap-warning-with-env-check-test.js @@ -36,7 +36,7 @@ describe('wrap-warning-with-env-check', () => { it('should wrap warning calls', () => { compare( "warning(condition, 'a %s b', 'c');", - "__DEV__ ? warning(condition, 'a %s b', 'c') : void 0;" + "__DEV__ ? !condition ? warning(false, 'a %s b', 'c') : void 0 : void 0;" ); }); diff --git a/scripts/rollup/plugins/wrap-warning-with-env-check.js b/scripts/babel/wrap-warning-with-env-check.js similarity index 63% rename from scripts/rollup/plugins/wrap-warning-with-env-check.js rename to scripts/babel/wrap-warning-with-env-check.js index 1ab26a6b9aed5..e760854ab2506 100644 --- a/scripts/rollup/plugins/wrap-warning-with-env-check.js +++ b/scripts/babel/wrap-warning-with-env-check.js @@ -25,8 +25,6 @@ module.exports = function(babel, options) { } if (path.get('callee').isIdentifier({name: 'warning'})) { - node[SEEN_SYMBOL] = true; - // Turns this code: // // warning(condition, argument, argument); @@ -34,14 +32,28 @@ module.exports = function(babel, options) { // into this: // // if (__DEV__) { - // warning(condition, argument, argument); + // if (!condition) { + // warning(false, argument, argument); + // } // } // - // The goal is to strip out warning calls entirely in production. + // The goal is to strip out warning calls entirely in production + // and to avoid evaluating the arguments in development. + const condition = node.arguments[0]; + const newNode = t.callExpression( + node.callee, + [t.booleanLiteral(false)].concat(node.arguments.slice(1)) + ); + newNode[SEEN_SYMBOL] = true; path.replaceWith( t.ifStatement( DEV_EXPRESSION, - t.blockStatement([t.expressionStatement(node)]) + t.blockStatement([ + t.ifStatement( + t.unaryExpression('!', condition), + t.expressionStatement(newNode) + ), + ]) ) ); } diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index fc2782b0ca53f..631625a37582d 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -17,6 +17,9 @@ const pathToBabel = path.join( const pathToBabelPluginDevWithCode = require.resolve( '../error-codes/replace-invariant-error-codes' ); +const pathToBabelPluginWrapWarning = require.resolve( + '../babel/wrap-warning-with-env-check' +); const pathToBabelPluginAsyncToGenerator = require.resolve( 'babel-plugin-transform-async-to-generator' ); @@ -29,6 +32,8 @@ const babelOptions = { require.resolve('babel-plugin-transform-es2015-modules-commonjs'), pathToBabelPluginDevWithCode, + pathToBabelPluginWrapWarning, + // Keep stacks detailed in tests. // Don't put this in .babelrc so that we don't embed filenames // into ReactART builds that include JSX. @@ -75,6 +80,7 @@ module.exports = { pathToBabel, pathToBabelrc, pathToBabelPluginDevWithCode, + pathToBabelPluginWrapWarning, pathToErrorCodes, ]), }; diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index a3b9d13fac4fd..30bfc8ab13609 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -87,7 +87,7 @@ function getBabelConfig(updateBabelOptions, bundleType, filename) { return Object.assign({}, options, { plugins: options.plugins.concat([ // Wrap warning() calls in a __DEV__ check so they are stripped from production. - require('./plugins/wrap-warning-with-env-check'), + require('../babel/wrap-warning-with-env-check'), ]), }); case UMD_DEV: @@ -101,7 +101,7 @@ function getBabelConfig(updateBabelOptions, bundleType, filename) { // Minify invariant messages require('../error-codes/replace-invariant-error-codes'), // Wrap warning() calls in a __DEV__ check so they are stripped from production. - require('./plugins/wrap-warning-with-env-check'), + require('../babel/wrap-warning-with-env-check'), ]), }); default: