From 05dc814cf061796c54e3aab7dd18a1b54615fc6b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 30 Sep 2019 11:14:51 -0700 Subject: [PATCH] Remove IIFE wrappers from dev invariant checks (#16963) The error transform works by replacing calls to `invariant` with an `if` statement. Since we're replacing a call expression with a statement, Babel wraps the new statement in an immediately-invoked function expression (IIFE). This wrapper is unnecessary in practice because our `invariant` calls are always part of their own expression statement. In the production bundle, the function wrappers are removed by Closure. But they remain in the development bundles. This commit updates the transform to confirm that an `invariant` call expression's parent node is an expression statement. (If not, it throws a transform error.) Then, it replaces the expression statement instead of the expression itself, effectively removing the extraneous IIFE wrapper. --- .../transform-error-messages.js.snap | 50 ++++++++----------- .../__tests__/transform-error-messages.js | 9 ++++ .../error-codes/transform-error-messages.js | 12 ++++- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap index b70498c73b0a..c8b027efe193 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -29,15 +29,13 @@ exports[`error transform should replace simple invariant calls 1`] = ` import _ReactError from \\"shared/ReactError\\"; import invariant from 'shared/invariant'; -(function () { - if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Do not override existing functions.\\")); - } else { - throw _ReactErrorProd(Error(16)); - } +if (!condition) { + if (__DEV__) { + throw _ReactError(Error(\\"Do not override existing functions.\\")); + } else { + throw _ReactErrorProd(Error(16)); } -})();" +}" `; exports[`error transform should support invariant calls with a concatenated template string and args 1`] = ` @@ -45,15 +43,13 @@ exports[`error transform should support invariant calls with a concatenated temp import _ReactError from \\"shared/ReactError\\"; import invariant from 'shared/invariant'; -(function () { - if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Expected a component class, got \\" + Foo + \\".\\" + Bar)); - } else { - throw _ReactErrorProd(Error(18), Foo, Bar); - } +if (!condition) { + if (__DEV__) { + throw _ReactError(Error(\\"Expected a component class, got \\" + Foo + \\".\\" + Bar)); + } else { + throw _ReactErrorProd(Error(18), Foo, Bar); } -})();" +}" `; exports[`error transform should support invariant calls with args 1`] = ` @@ -61,24 +57,20 @@ exports[`error transform should support invariant calls with args 1`] = ` import _ReactError from \\"shared/ReactError\\"; import invariant from 'shared/invariant'; -(function () { - if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Expected \\" + foo + \\" target to be an array; got \\" + bar)); - } else { - throw _ReactErrorProd(Error(7), foo, bar); - } +if (!condition) { + if (__DEV__) { + throw _ReactError(Error(\\"Expected \\" + foo + \\" target to be an array; got \\" + bar)); + } else { + throw _ReactErrorProd(Error(7), foo, bar); } -})();" +}" `; exports[`error transform should support noMinify option 1`] = ` "import _ReactError from \\"shared/ReactError\\"; import invariant from 'shared/invariant'; -(function () { - if (!condition) { - throw _ReactError(Error(\\"Do not override existing functions.\\")); - } -})();" +if (!condition) { + throw _ReactError(Error(\\"Do not override existing functions.\\")); +}" `; diff --git a/scripts/error-codes/__tests__/transform-error-messages.js b/scripts/error-codes/__tests__/transform-error-messages.js index 97d9011a431f..4da00ac656b9 100644 --- a/scripts/error-codes/__tests__/transform-error-messages.js +++ b/scripts/error-codes/__tests__/transform-error-messages.js @@ -37,6 +37,15 @@ invariant(condition, 'Do not override existing functions.'); ).toMatchSnapshot(); }); + it('should throw if invariant is not in an expression statement', () => { + expect(() => { + transform(` +import invariant from 'shared/invariant'; +cond && invariant(condition, 'Do not override existing functions.'); +`); + }).toThrow('invariant() cannot be called from expression context'); + }); + it('should support invariant calls with args', () => { expect( transform(` diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index 193db9b2dadc..70a1fc5a2e9b 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -64,6 +64,14 @@ module.exports = function(babel) { ]) ); + const parentStatementPath = path.parentPath; + if (parentStatementPath.type !== 'ExpressionStatement') { + throw path.buildCodeFrameError( + 'invariant() cannot be called from expression context. Move ' + + 'the call to its own statement.' + ); + } + if (noMinify) { // Error minification is disabled for this build. // @@ -71,7 +79,7 @@ module.exports = function(babel) { // if (!condition) { // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); // } - path.replaceWith( + parentStatementPath.replaceWith( t.ifStatement( t.unaryExpression('!', condition), t.blockStatement([devThrow]) @@ -138,7 +146,7 @@ module.exports = function(babel) { // throw ReactErrorProd(Error(ERR_CODE), adj, noun); // } // } - path.replaceWith( + parentStatementPath.replaceWith( t.ifStatement( t.unaryExpression('!', condition), t.blockStatement([