Skip to content

Commit

Permalink
Don't run error transform in development
Browse files Browse the repository at this point in the history
We used to run the error transform in both production and development,
because in development it was used to convert `invariant` calls into
throw statements.

Now that don't use `invariant` anymore, we only have to run the
transform for production builds.
  • Loading branch information
acdlite committed Oct 6, 2021
1 parent 5b85a39 commit b948f41
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 287 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,6 @@ exports[`error transform handles escaped backticks in template string 1`] = `
Error(__DEV__ ? \\"Expected \`\\" + listener + \\"\` listener to be a function, instead got a value of \`\\" + type + \\"\` type.\\" : _formatProdErrorMessage(231, listener, type));"
`;

exports[`error transform should correctly transform invariants that are not in the error codes map 1`] = `
"import invariant from 'shared/invariant';
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/
if (!condition) {
throw Error(\\"This is not a real error message.\\");
}"
`;

exports[`error transform should handle escaped characters 1`] = `
"import invariant from 'shared/invariant';
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/
if (!condition) {
throw Error(\\"What's up?\\");
}"
`;

exports[`error transform should not touch other calls or new expressions 1`] = `
"new NotAnError();
NotAnError();"
Expand All @@ -48,17 +30,6 @@ exports[`error transform should replace error constructors 1`] = `
Error(__DEV__ ? 'Do not override existing functions.' : _formatProdErrorMessage(16));"
`;

exports[`error transform should replace simple invariant calls 1`] = `
"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
import invariant from 'shared/invariant';
if (!condition) {
{
throw Error(__DEV__ ? \\"Do not override existing functions.\\" : _formatProdErrorMessage(16));
}
}"
`;

exports[`error transform should support error constructors with concatenated messages 1`] = `
"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
Error(__DEV__ ? \\"Expected \\" + foo + \\" target to \\" + (\\"be an array; got \\" + bar) : _formatProdErrorMessage(7, foo, bar));"
Expand All @@ -73,33 +44,3 @@ exports[`error transform should support interpolating arguments with template st
"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
Error(__DEV__ ? \\"Expected \\" + foo + \\" target to be an array; got \\" + bar : _formatProdErrorMessage(7, foo, bar));"
`;

exports[`error transform should support invariant calls with a concatenated template string and args 1`] = `
"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
import invariant from 'shared/invariant';
if (!condition) {
{
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 _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
import invariant from 'shared/invariant';
if (!condition) {
{
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 invariant from 'shared/invariant';
if (!condition) {
throw Error(\\"Do not override existing functions.\\");
}"
`;
66 changes: 0 additions & 66 deletions scripts/error-codes/__tests__/transform-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,72 +28,6 @@ describe('error transform', () => {
process.env.NODE_ENV = oldEnv;
});

it('should replace simple invariant calls', () => {
expect(
transform(`
import invariant from 'shared/invariant';
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(`
import invariant from 'shared/invariant';
invariant(condition, 'Expected %s target to be an array; got %s', foo, bar);
`)
).toMatchSnapshot();
});

it('should support invariant calls with a concatenated template string and args', () => {
expect(
transform(`
import invariant from 'shared/invariant';
invariant(condition, 'Expected a component class, ' + 'got %s.' + '%s', Foo, Bar);
`)
).toMatchSnapshot();
});

it('should correctly transform invariants that are not in the error codes map', () => {
expect(
transform(`
import invariant from 'shared/invariant';
invariant(condition, 'This is not a real error message.');
`)
).toMatchSnapshot();
});

it('should handle escaped characters', () => {
expect(
transform(`
import invariant from 'shared/invariant';
invariant(condition, 'What\\'s up?');
`)
).toMatchSnapshot();
});

it('should support noMinify option', () => {
expect(
transform(
`
import invariant from 'shared/invariant';
invariant(condition, 'Do not override existing functions.');
`,
{noMinify: true}
)
).toMatchSnapshot();
});

it('should replace error constructors', () => {
expect(
transform(`
Expand Down
147 changes: 6 additions & 141 deletions scripts/error-codes/transform-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
'use strict';

const fs = require('fs');
const {
evalStringConcat,
evalStringAndTemplateConcat,
} = require('../shared/evalToString');
const {evalStringAndTemplateConcat} = require('../shared/evalToString');
const invertObject = require('./invertObject');
const helperModuleImports = require('@babel/helper-module-imports');

Expand All @@ -27,7 +24,7 @@ module.exports = function(babel) {
// in production.
const DEV_EXPRESSION = t.identifier('__DEV__');

function CallOrNewExpression(path, file) {
function ErrorCallExpression(path, file) {
// Turns this code:
//
// new Error(`A ${adj} message that contains ${noun}`);
Expand Down Expand Up @@ -129,148 +126,16 @@ module.exports = function(babel) {
return {
visitor: {
NewExpression(path, file) {
const noMinify = file.opts.noMinify;
if (!noMinify && path.get('callee').isIdentifier({name: 'Error'})) {
CallOrNewExpression(path, file);
if (path.get('callee').isIdentifier({name: 'Error'})) {
ErrorCallExpression(path, file);
}
},

CallExpression(path, file) {
const node = path.node;
const noMinify = file.opts.noMinify;

if (!noMinify && path.get('callee').isIdentifier({name: 'Error'})) {
CallOrNewExpression(path, file);
if (path.get('callee').isIdentifier({name: 'Error'})) {
ErrorCallExpression(path, file);
return;
}

if (path.get('callee').isIdentifier({name: 'invariant'})) {
// Turns this code:
//
// invariant(condition, 'A %s message that contains %s', adj, noun);
//
// into this:
//
// if (!condition) {
// 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
// string) that references a verbose error message. The mapping is
// stored in `scripts/error-codes/codes.json`.
const condition = node.arguments[0];
const errorMsgLiteral = evalStringConcat(node.arguments[1]);
const errorMsgExpressions = Array.from(node.arguments.slice(2));
const errorMsgQuasis = errorMsgLiteral
.split('%s')
.map(raw => t.templateElement({raw, cooked: String.raw({raw})}));

// Outputs:
// `A ${adj} message that contains ${noun}`;
const devMessage = t.templateLiteral(
errorMsgQuasis,
errorMsgExpressions
);

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.
//
// Outputs:
// if (!condition) {
// throw Error(`A ${adj} message that contains ${noun}`);
// }
const errorCallNode = t.callExpression(t.identifier('Error'), [
devMessage,
]);
errorCallNode[SEEN_SYMBOL] = true;
parentStatementPath.replaceWith(
t.ifStatement(
t.unaryExpression('!', condition),
t.blockStatement([t.throwStatement(errorCallNode)])
)
);

return;
}

let prodErrorId = errorMap[errorMsgLiteral];

if (prodErrorId === undefined) {
// There is no error code for this message. Add an inline comment
// that flags this as an unminified error. This allows the build
// to proceed, while also allowing a post-build linter to detect it.
//
// Outputs:
// /* FIXME (minify-errors-in-prod): Unminified error message in production build! */
// if (!condition) {
// throw Error(`A ${adj} message that contains ${noun}`);
// }
const errorCall = t.callExpression(t.identifier('Error'), [
devMessage,
]);
errorCall[SEEN_SYMBOL] = true;
parentStatementPath.replaceWith(
t.ifStatement(
t.unaryExpression('!', condition),
t.blockStatement([t.throwStatement(errorCall)])
)
);
parentStatementPath.addComment(
'leading',
'FIXME (minify-errors-in-prod): Unminified error message in production build!'
);
return;
}
prodErrorId = parseInt(prodErrorId, 10);

// Import formatProdErrorMessage
const formatProdErrorMessageIdentifier = helperModuleImports.addDefault(
path,
'shared/formatProdErrorMessage',
{nameHint: 'formatProdErrorMessage'}
);

// Outputs:
// formatProdErrorMessage(ERR_CODE, adj, noun);
const prodMessage = t.callExpression(
formatProdErrorMessageIdentifier,
[t.numericLiteral(prodErrorId), ...errorMsgExpressions]
);

const errorCall = t.callExpression(t.identifier('Error'), [
t.conditionalExpression(DEV_EXPRESSION, devMessage, prodMessage),
]);
errorCall[SEEN_SYMBOL] = true;

// Outputs:
// 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.blockStatement([t.throwStatement(errorCall)]),
])
)
);
}
},
},
};
Expand Down
31 changes: 10 additions & 21 deletions scripts/rollup/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,26 +176,17 @@ function getBabelConfig(
if (updateBabelOptions) {
options = updateBabelOptions(options);
}
// Controls whether to replace error messages with error codes in production.
// By default, error messages are replaced in production.
if (!isDevelopment && bundle.minifyWithProdErrorCodes !== false) {
Object.assign({}, options, {
plugins: options.plugins.concat([
[require('../error-codes/transform-error-messages')],
]),
});
}

switch (bundleType) {
case FB_WWW_DEV:
case FB_WWW_PROD:
case FB_WWW_PROFILING:
case RN_OSS_DEV:
case RN_OSS_PROD:
case RN_OSS_PROFILING:
case RN_FB_DEV:
case RN_FB_PROD:
case RN_FB_PROFILING:
return Object.assign({}, options, {
plugins: options.plugins.concat([
[
require('../error-codes/transform-error-messages'),
// Controls whether to replace error messages with error codes
// in production. By default, error messages are replaced.
{noMinify: bundle.minifyWithProdErrorCodes === false},
],
]),
});
case UMD_DEV:
case UMD_PROD:
case UMD_PROFILING:
Expand All @@ -206,8 +197,6 @@ function getBabelConfig(
plugins: options.plugins.concat([
// Use object-assign polyfill in open source
path.resolve('./scripts/babel/transform-object-assign-require'),
// Minify invariant messages
require('../error-codes/transform-error-messages'),
]),
});
default:
Expand Down

0 comments on commit b948f41

Please sign in to comment.