From 75955bf1d7ff6c2c1f4052f4a84dd2ce6944c62e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 11 Oct 2019 09:10:40 -0700 Subject: [PATCH] Pass prod error messages directly to constructor (#17063) * Remove "Invariant Violation" from dev errors When I made the change to compile `invariant` to throw expressions, I left a small runtime to set the error's `name` property to "Invariant Violation" to maintain the existing behavior. I think we can remove it. The argument for keeping it is to preserve continuity in error logs, but this only affects development errors, anyway: production error messages are replaced with error codes. * Pass prod error messages directly to constructor Updates the `invariant` transform to pass an error message string directly to the Error constructor, instead of mutating the message property. Turns this code: ```js invariant(condition, 'A %s message that contains %s', adj, noun); ``` into this: ```js if (!condition) { throw Error( __DEV__ ? `A ${adj} message that contains ${noun}` : formatProdErrorMessage(ERR_CODE, adj, noun) ); } ``` --- .../ReactNativeError-test.internal.js | 2 +- packages/shared/ReactError.js | 18 ---- .../__tests__/ReactError-test.internal.js | 10 +- .../__tests__/ReactErrorProd-test.internal.js | 16 +-- ...ErrorProd.js => formatProdErrorMessage.js} | 15 ++- .../transform-error-messages.js.snap | 54 ++++------- .../error-codes/transform-error-messages.js | 97 +++++++++---------- 7 files changed, 81 insertions(+), 131 deletions(-) delete mode 100644 packages/shared/ReactError.js rename packages/shared/{ReactErrorProd.js => formatProdErrorMessage.js} (68%) diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js index f6a78667eda36..96d6803ceaae6 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js @@ -40,7 +40,7 @@ describe('ReactNativeError', () => { throw new Error(e.toString()); } }).toThrow( - 'Invariant Violation: View config getter callback for component `View` must be a function (received `null`)', + 'View config getter callback for component `View` must be a function (received `null`)', ); }); diff --git a/packages/shared/ReactError.js b/packages/shared/ReactError.js deleted file mode 100644 index 0a5b1ed222d8c..0000000000000 --- a/packages/shared/ReactError.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - */ - -// Do not require this module directly! Use normal `invariant` calls with -// template literal strings. The messages will be converted to ReactError during -// build, and in production they will be minified. - -function ReactError(error) { - error.name = 'Invariant Violation'; - return error; -} - -export default ReactError; diff --git a/packages/shared/__tests__/ReactError-test.internal.js b/packages/shared/__tests__/ReactError-test.internal.js index a8b9bcb557ae5..cd9fc5697f86d 100644 --- a/packages/shared/__tests__/ReactError-test.internal.js +++ b/packages/shared/__tests__/ReactError-test.internal.js @@ -37,15 +37,7 @@ describe('ReactError', () => { }); if (__DEV__) { - it('should throw errors whose name is "Invariant Violation"', () => { - let error; - try { - React.useState(); - } catch (e) { - error = e; - } - expect(error.name).toEqual('Invariant Violation'); - }); + it("empty test so Jest doesn't complain", () => {}); } else { it('should error with minified error code', () => { expect(() => ReactDOM.render('Hi', null)).toThrowError( diff --git a/packages/shared/__tests__/ReactErrorProd-test.internal.js b/packages/shared/__tests__/ReactErrorProd-test.internal.js index 62d6da32c5c2d..27679bc529920 100644 --- a/packages/shared/__tests__/ReactErrorProd-test.internal.js +++ b/packages/shared/__tests__/ReactErrorProd-test.internal.js @@ -8,7 +8,7 @@ */ 'use strict'; -let ReactErrorProd; +let formatProdErrorMessage; describe('ReactErrorProd', () => { let globalErrorMock; @@ -25,7 +25,7 @@ describe('ReactErrorProd', () => { expect(typeof global.Error).toBe('function'); } jest.resetModules(); - ReactErrorProd = require('shared/ReactErrorProd').default; + formatProdErrorMessage = require('shared/formatProdErrorMessage').default; }); afterEach(() => { @@ -35,27 +35,21 @@ describe('ReactErrorProd', () => { }); it('should throw with the correct number of `%s`s in the URL', () => { - expect(function() { - throw ReactErrorProd(Error(124), 'foo', 'bar'); - }).toThrowError( + expect(formatProdErrorMessage(124, 'foo', 'bar')).toEqual( 'Minified React error #124; visit ' + 'https://reactjs.org/docs/error-decoder.html?invariant=124&args[]=foo&args[]=bar' + ' for the full message or use the non-minified dev environment' + ' for full errors and additional helpful warnings.', ); - expect(function() { - throw ReactErrorProd(Error(20)); - }).toThrowError( + expect(formatProdErrorMessage(20)).toEqual( 'Minified React error #20; visit ' + 'https://reactjs.org/docs/error-decoder.html?invariant=20' + ' for the full message or use the non-minified dev environment' + ' for full errors and additional helpful warnings.', ); - expect(function() { - throw ReactErrorProd(Error(77), '
', '&?bar'); - }).toThrowError( + expect(formatProdErrorMessage(77, '
', '&?bar')).toEqual( 'Minified React error #77; visit ' + 'https://reactjs.org/docs/error-decoder.html?invariant=77&args[]=%3Cdiv%3E&args[]=%26%3Fbar' + ' for the full message or use the non-minified dev environment' + diff --git a/packages/shared/ReactErrorProd.js b/packages/shared/formatProdErrorMessage.js similarity index 68% rename from packages/shared/ReactErrorProd.js rename to packages/shared/formatProdErrorMessage.js index b909e2180809e..e315ed83155c3 100644 --- a/packages/shared/ReactErrorProd.js +++ b/packages/shared/formatProdErrorMessage.js @@ -7,20 +7,19 @@ */ // Do not require this module directly! Use normal `invariant` calls with -// template literal strings. The messages will be converted to ReactError during -// build, and in production they will be minified. +// template literal strings. The messages will be replaced with error codes +// during build. -function ReactErrorProd(error) { - const code = error.message; +function formatProdErrorMessage(code) { let url = 'https://reactjs.org/docs/error-decoder.html?invariant=' + code; for (let i = 1; i < arguments.length; i++) { url += '&args[]=' + encodeURIComponent(arguments[i]); } - error.message = + return ( `Minified React error #${code}; visit ${url} for the full message or ` + 'use the non-minified dev environment for full errors and additional ' + - 'helpful warnings. '; - return error; + 'helpful warnings.' + ); } -export default ReactErrorProd; +export default formatProdErrorMessage; 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 c8b027efe1937..f93b31a659dd2 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -1,76 +1,60 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`error transform should correctly transform invariants that are not in the error codes map 1`] = ` -"import _ReactError from \\"shared/ReactError\\"; -import invariant from 'shared/invariant'; +"import invariant from 'shared/invariant'; /*FIXME (minify-errors-in-prod): Unminified error message in production build!*/ -(function () { - if (!condition) { - throw _ReactError(Error(\\"This is not a real error message.\\")); - } -})();" +if (!condition) { + throw Error(\\"This is not a real error message.\\"); +}" `; exports[`error transform should handle escaped characters 1`] = ` -"import _ReactError from \\"shared/ReactError\\"; -import invariant from 'shared/invariant'; +"import invariant from 'shared/invariant'; /*FIXME (minify-errors-in-prod): Unminified error message in production build!*/ -(function () { - if (!condition) { - throw _ReactError(Error(\\"What's up?\\")); - } -})();" +if (!condition) { + throw Error(\\"What's up?\\"); +}" `; exports[`error transform should replace simple invariant calls 1`] = ` -"import _ReactErrorProd from \\"shared/ReactErrorProd\\"; -import _ReactError from \\"shared/ReactError\\"; +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; import invariant from 'shared/invariant'; if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Do not override existing functions.\\")); - } else { - throw _ReactErrorProd(Error(16)); + { + throw Error(__DEV__ ? \\"Do not override existing functions.\\" : _formatProdErrorMessage(16)); } }" `; exports[`error transform should support invariant calls with a concatenated template string and args 1`] = ` -"import _ReactErrorProd from \\"shared/ReactErrorProd\\"; -import _ReactError from \\"shared/ReactError\\"; +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; import invariant from 'shared/invariant'; if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Expected a component class, got \\" + Foo + \\".\\" + Bar)); - } else { - throw _ReactErrorProd(Error(18), Foo, Bar); + { + throw Error(__DEV__ ? \\"Expected a component class, got \\" + Foo + \\".\\" + Bar : _formatProdErrorMessage(18, Foo, Bar)); } }" `; exports[`error transform should support invariant calls with args 1`] = ` -"import _ReactErrorProd from \\"shared/ReactErrorProd\\"; -import _ReactError from \\"shared/ReactError\\"; +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; import invariant from 'shared/invariant'; if (!condition) { - if (__DEV__) { - throw _ReactError(Error(\\"Expected \\" + foo + \\" target to be an array; got \\" + bar)); - } else { - throw _ReactErrorProd(Error(7), foo, bar); + { + throw Error(__DEV__ ? \\"Expected \\" + foo + \\" target to be an array; got \\" + bar : _formatProdErrorMessage(7, foo, bar)); } }" `; exports[`error transform should support noMinify option 1`] = ` -"import _ReactError from \\"shared/ReactError\\"; -import invariant from 'shared/invariant'; +"import invariant from 'shared/invariant'; if (!condition) { - throw _ReactError(Error(\\"Do not override existing functions.\\")); + throw Error(\\"Do not override existing functions.\\"); }" `; diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index 70a1fc5a2e9bb..6ebb68731f860 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -29,11 +29,11 @@ module.exports = function(babel) { // into this: // // if (!condition) { - // if (__DEV__) { - // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); - // } else { - // throw ReactErrorProd(Error(ERR_CODE), adj, noun); - // } + // throw Error( + // __DEV__ + // ? `A ${adj} message that contains ${noun}` + // : formatProdErrorMessage(ERR_CODE, adj, noun) + // ); // } // // where ERR_CODE is an error code: a unique identifier (a number @@ -46,22 +46,11 @@ module.exports = function(babel) { .split('%s') .map(raw => t.templateElement({raw, cooked: String.raw({raw})})); - const reactErrorIdentfier = helperModuleImports.addDefault( - path, - 'shared/ReactError', - { - nameHint: 'ReactError', - } - ); - // Outputs: - // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); - const devThrow = t.throwStatement( - t.callExpression(reactErrorIdentfier, [ - t.callExpression(t.identifier('Error'), [ - t.templateLiteral(errorMsgQuasis, errorMsgExpressions), - ]), - ]) + // `A ${adj} message that contains ${noun}`; + const devMessage = t.templateLiteral( + errorMsgQuasis, + errorMsgExpressions ); const parentStatementPath = path.parentPath; @@ -77,12 +66,16 @@ module.exports = function(babel) { // // Outputs: // if (!condition) { - // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); + // throw Error(`A ${adj} message that contains ${noun}`); // } parentStatementPath.replaceWith( t.ifStatement( t.unaryExpression('!', condition), - t.blockStatement([devThrow]) + t.blockStatement([ + t.throwStatement( + t.callExpression(t.identifier('Error'), [devMessage]) + ), + ]) ) ); return; @@ -104,15 +97,19 @@ module.exports = function(babel) { // Outputs: // /* FIXME (minify-errors-in-prod): Unminified error message in production build! */ // if (!condition) { - // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); + // throw Error(`A ${adj} message that contains ${noun}`); // } - path.replaceWith( + parentStatementPath.replaceWith( t.ifStatement( t.unaryExpression('!', condition), - t.blockStatement([devThrow]) + t.blockStatement([ + t.throwStatement( + t.callExpression(t.identifier('Error'), [devMessage]) + ), + ]) ) ); - path.addComment( + parentStatementPath.addComment( 'leading', 'FIXME (minify-errors-in-prod): Unminified error message in production build!' ); @@ -121,40 +118,42 @@ module.exports = function(babel) { prodErrorId = parseInt(prodErrorId, 10); // Import ReactErrorProd - const reactErrorProdIdentfier = helperModuleImports.addDefault( + const formatProdErrorMessageIdentifier = helperModuleImports.addDefault( path, - 'shared/ReactErrorProd', - {nameHint: 'ReactErrorProd'} + 'shared/formatProdErrorMessage', + {nameHint: 'formatProdErrorMessage'} ); // Outputs: - // throw ReactErrorProd(Error(ERR_CODE), adj, noun); - const prodThrow = t.throwStatement( - t.callExpression(reactErrorProdIdentfier, [ - t.callExpression(t.identifier('Error'), [ - t.numericLiteral(prodErrorId), - ]), - ...errorMsgExpressions, - ]) + // formatProdErrorMessage(ERR_CODE, adj, noun); + const prodMessage = t.callExpression( + formatProdErrorMessageIdentifier, + [t.numericLiteral(prodErrorId), ...errorMsgExpressions] ); // Outputs: - // if (!condition) { - // if (__DEV__) { - // throw ReactError(Error(`A ${adj} message that contains ${noun}`)); - // } else { - // throw ReactErrorProd(Error(ERR_CODE), adj, noun); - // } - // } + // if (!condition) { + // throw Error( + // __DEV__ + // ? `A ${adj} message that contains ${noun}` + // : formatProdErrorMessage(ERR_CODE, adj, noun) + // ); + // } parentStatementPath.replaceWith( t.ifStatement( t.unaryExpression('!', condition), t.blockStatement([ - t.ifStatement( - DEV_EXPRESSION, - t.blockStatement([devThrow]), - t.blockStatement([prodThrow]) - ), + t.blockStatement([ + t.throwStatement( + t.callExpression(t.identifier('Error'), [ + t.conditionalExpression( + DEV_EXPRESSION, + devMessage, + prodMessage + ), + ]) + ), + ]), ]) ) );