Skip to content

Commit a81d95c

Browse files
committed
Output FIXME during build for unminified errors
The invariant Babel transform used to output a FIXME comment if it could not find a matching error code. This could happen if there were a configuration mistake that caused an unminified message to slip through. Linting the compiled bundles is the most reliable way to do it because there's not a one-to-one mapping between source modules and bundles. For example, the same source module may appear in multiple bundles, some which are minified and others which aren't. This updates the transform to output the same messages for Error calls. The source lint rule is still useful for catching mistakes during development, to prompt you to update the error codes map before pushing the PR to CI.
1 parent 6ecad79 commit a81d95c

File tree

3 files changed

+89
-30
lines changed

3 files changed

+89
-30
lines changed

scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@ exports[`error transform should not touch other calls or new expressions 1`] = `
2828
NotAnError();"
2929
`;
3030

31+
exports[`error transform should output FIXME for errors that don't have a matching error code 1`] = `
32+
"/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/
33+
Error(condition, 'This is not a real error message.');"
34+
`;
35+
36+
exports[`error transform should output FIXME for errors that don't have a matching error code, unless opted out with a comment 1`] = `
37+
"// eslint-disable-next-line react-internal/prod-error-codes
38+
Error(condition, 'This is not a real error message.');"
39+
`;
40+
3141
exports[`error transform should replace error constructors (no new) 1`] = `
3242
"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
3343
Error(__DEV__ ? 'Do not override existing functions.' : _formatProdErrorMessage(16));"

scripts/error-codes/__tests__/transform-error-messages.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,31 @@ Error('Do not override existing functions.');
110110
).toMatchSnapshot();
111111
});
112112

113+
it("should output FIXME for errors that don't have a matching error code", () => {
114+
expect(
115+
transform(`
116+
Error(condition, 'This is not a real error message.');
117+
`)
118+
).toMatchSnapshot();
119+
});
120+
121+
it(
122+
"should output FIXME for errors that don't have a matching error " +
123+
'code, unless opted out with a comment',
124+
() => {
125+
// TODO: Since this only detects one of many ways to disable a lint
126+
// rule, we should instead search for a custom directive (like
127+
// no-minify-errors) instead of ESLint. Will need to update our lint
128+
// rule to recognize the same directive.
129+
expect(
130+
transform(`
131+
// eslint-disable-next-line react-internal/prod-error-codes
132+
Error(condition, 'This is not a real error message.');
133+
`)
134+
).toMatchSnapshot();
135+
}
136+
);
137+
113138
it('should not touch other calls or new expressions', () => {
114139
expect(
115140
transform(`

scripts/error-codes/transform-error-messages.js

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,38 @@ module.exports = function(babel) {
6262

6363
let prodErrorId = errorMap[errorMsgLiteral];
6464
if (prodErrorId === undefined) {
65-
// There is no error code for this message. We use a lint rule to
66-
// enforce that messages can be minified, so assume this is
67-
// intentional and exit gracefully.
65+
// There is no error code for this message. Add an inline comment
66+
// that flags this as an unminified error. This allows the build
67+
// to proceed, while also allowing a post-build linter to detect it.
68+
//
69+
// Outputs:
70+
// /* FIXME (minify-errors-in-prod): Unminified error message in production build! */
71+
// if (!condition) {
72+
// throw Error(`A ${adj} message that contains ${noun}`);
73+
// }
74+
75+
const leadingComments = path.parentPath.node.leadingComments;
76+
if (leadingComments !== undefined) {
77+
for (let i = 0; i < leadingComments.length; i++) {
78+
// TODO: Since this only detects one of many ways to disable a lint
79+
// rule, we should instead search for a custom directive (like
80+
// no-minify-errors) instead of ESLint. Will need to update our lint
81+
// rule to recognize the same directive.
82+
const commentText = leadingComments[i].value;
83+
if (
84+
commentText.includes(
85+
'eslint-disable-next-line react-internal/prod-error-codes'
86+
)
87+
) {
88+
return;
89+
}
90+
}
91+
}
92+
93+
path.addComment(
94+
'leading',
95+
'FIXME (minify-errors-in-prod): Unminified error message in production build!'
96+
);
6897
return;
6998
}
7099
prodErrorId = parseInt(prodErrorId, 10);
@@ -89,12 +118,11 @@ module.exports = function(babel) {
89118
// ? `A ${adj} message that contains ${noun}`
90119
// : formatProdErrorMessage(ERR_CODE, adj, noun)
91120
// );
92-
path.replaceWith(t.callExpression(t.identifier('Error'), [prodMessage]));
93-
path.replaceWith(
94-
t.callExpression(t.identifier('Error'), [
95-
t.conditionalExpression(DEV_EXPRESSION, errorMsgNode, prodMessage),
96-
])
97-
);
121+
const newErrorCall = t.callExpression(t.identifier('Error'), [
122+
t.conditionalExpression(DEV_EXPRESSION, errorMsgNode, prodMessage),
123+
]);
124+
newErrorCall[SEEN_SYMBOL] = true;
125+
path.replaceWith(newErrorCall);
98126
}
99127

100128
return {
@@ -162,16 +190,17 @@ module.exports = function(babel) {
162190
// if (!condition) {
163191
// throw Error(`A ${adj} message that contains ${noun}`);
164192
// }
193+
const errorCallNode = t.callExpression(t.identifier('Error'), [
194+
devMessage,
195+
]);
196+
errorCallNode[SEEN_SYMBOL] = true;
165197
parentStatementPath.replaceWith(
166198
t.ifStatement(
167199
t.unaryExpression('!', condition),
168-
t.blockStatement([
169-
t.throwStatement(
170-
t.callExpression(t.identifier('Error'), [devMessage])
171-
),
172-
])
200+
t.blockStatement([t.throwStatement(errorCallNode)])
173201
)
174202
);
203+
175204
return;
176205
}
177206

@@ -187,14 +216,14 @@ module.exports = function(babel) {
187216
// if (!condition) {
188217
// throw Error(`A ${adj} message that contains ${noun}`);
189218
// }
219+
const errorCall = t.callExpression(t.identifier('Error'), [
220+
devMessage,
221+
]);
222+
errorCall[SEEN_SYMBOL] = true;
190223
parentStatementPath.replaceWith(
191224
t.ifStatement(
192225
t.unaryExpression('!', condition),
193-
t.blockStatement([
194-
t.throwStatement(
195-
t.callExpression(t.identifier('Error'), [devMessage])
196-
),
197-
])
226+
t.blockStatement([t.throwStatement(errorCall)])
198227
)
199228
);
200229
parentStatementPath.addComment(
@@ -219,6 +248,11 @@ module.exports = function(babel) {
219248
[t.numericLiteral(prodErrorId), ...errorMsgExpressions]
220249
);
221250

251+
const errorCall = t.callExpression(t.identifier('Error'), [
252+
t.conditionalExpression(DEV_EXPRESSION, devMessage, prodMessage),
253+
]);
254+
errorCall[SEEN_SYMBOL] = true;
255+
222256
// Outputs:
223257
// if (!condition) {
224258
// throw Error(
@@ -231,17 +265,7 @@ module.exports = function(babel) {
231265
t.ifStatement(
232266
t.unaryExpression('!', condition),
233267
t.blockStatement([
234-
t.blockStatement([
235-
t.throwStatement(
236-
t.callExpression(t.identifier('Error'), [
237-
t.conditionalExpression(
238-
DEV_EXPRESSION,
239-
devMessage,
240-
prodMessage
241-
),
242-
])
243-
),
244-
]),
268+
t.blockStatement([t.throwStatement(errorCall)]),
245269
])
246270
)
247271
);

0 commit comments

Comments
 (0)