Add a build step to hoist warning conditions (facebook#12537)

rhagigi · Apr 19, 2018 · a2530a1 · a2530a1
1 parent 3508776
commit a2530a1
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 8 deletions.
diff --git a/...sts__/wrap-warning-with-env-check-test.js → ...sts__/wrap-warning-with-env-check-test.js b/...sts__/wrap-warning-with-env-check-test.js → ...sts__/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/...up/plugins/wrap-warning-with-env-check.js → scripts/babel/wrap-warning-with-env-check.js b/...up/plugins/wrap-warning-with-env-check.js → scripts/babel/wrap-warning-with-env-check.js
@@ -25,23 +25,35 @@ module.exports = function(babel, options) {
           }
 
           if (path.get('callee').isIdentifier({name: 'warning'})) {
-            node[SEEN_SYMBOL] = true;
-
             // Turns this code:
             //
             // warning(condition, argument, argument);
             //
             // 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
@@ -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
@@ -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: