Skip to content

Commit

Permalink
Pass prod error messages directly to constructor (#17063)
Browse files Browse the repository at this point in the history
* 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)
  );
}
```
  • Loading branch information
acdlite authored Oct 11, 2019
1 parent 0ac8e56 commit 75955bf
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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`)',
);
});

Expand Down
18 changes: 0 additions & 18 deletions packages/shared/ReactError.js

This file was deleted.

10 changes: 1 addition & 9 deletions packages/shared/__tests__/ReactError-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 5 additions & 11 deletions packages/shared/__tests__/ReactErrorProd-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/
'use strict';

let ReactErrorProd;
let formatProdErrorMessage;

describe('ReactErrorProd', () => {
let globalErrorMock;
Expand All @@ -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(() => {
Expand All @@ -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), '<div>', '&?bar');
}).toThrowError(
expect(formatProdErrorMessage(77, '<div>', '&?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' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -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.\\");
}"
`;
97 changes: 48 additions & 49 deletions scripts/error-codes/transform-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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!'
);
Expand All @@ -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
),
])
),
]),
])
)
);
Expand Down

0 comments on commit 75955bf

Please sign in to comment.