Skip to content

Commit 5b85a39

Browse files
acdliteacdlite
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 5b85a39

File tree

3 files changed

+90
-30
lines changed

3 files changed

+90
-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: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,39 @@ 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 statementParent = path.getStatementParent();
76+
const leadingComments = statementParent.node.leadingComments;
77+
if (leadingComments !== undefined) {
78+
for (let i = 0; i < leadingComments.length; i++) {
79+
// TODO: Since this only detects one of many ways to disable a lint
80+
// rule, we should instead search for a custom directive (like
81+
// no-minify-errors) instead of ESLint. Will need to update our lint
82+
// rule to recognize the same directive.
83+
const commentText = leadingComments[i].value;
84+
if (
85+
commentText.includes(
86+
'eslint-disable-next-line react-internal/prod-error-codes'
87+
)
88+
) {
89+
return;
90+
}
91+
}
92+
}
93+
94+
statementParent.addComment(
95+
'leading',
96+
'FIXME (minify-errors-in-prod): Unminified error message in production build!'
97+
);
6898
return;
6999
}
70100
prodErrorId = parseInt(prodErrorId, 10);
@@ -89,12 +119,11 @@ module.exports = function(babel) {
89119
// ? `A ${adj} message that contains ${noun}`
90120
// : formatProdErrorMessage(ERR_CODE, adj, noun)
91121
// );
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-
);
122+
const newErrorCall = t.callExpression(t.identifier('Error'), [
123+
t.conditionalExpression(DEV_EXPRESSION, errorMsgNode, prodMessage),
124+
]);
125+
newErrorCall[SEEN_SYMBOL] = true;
126+
path.replaceWith(newErrorCall);
98127
}
99128

100129
return {
@@ -162,16 +191,17 @@ module.exports = function(babel) {
162191
// if (!condition) {
163192
// throw Error(`A ${adj} message that contains ${noun}`);
164193
// }
194+
const errorCallNode = t.callExpression(t.identifier('Error'), [
195+
devMessage,
196+
]);
197+
errorCallNode[SEEN_SYMBOL] = true;
165198
parentStatementPath.replaceWith(
166199
t.ifStatement(
167200
t.unaryExpression('!', condition),
168-
t.blockStatement([
169-
t.throwStatement(
170-
t.callExpression(t.identifier('Error'), [devMessage])
171-
),
172-
])
201+
t.blockStatement([t.throwStatement(errorCallNode)])
173202
)
174203
);
204+
175205
return;
176206
}
177207

@@ -187,14 +217,14 @@ module.exports = function(babel) {
187217
// if (!condition) {
188218
// throw Error(`A ${adj} message that contains ${noun}`);
189219
// }
220+
const errorCall = t.callExpression(t.identifier('Error'), [
221+
devMessage,
222+
]);
223+
errorCall[SEEN_SYMBOL] = true;
190224
parentStatementPath.replaceWith(
191225
t.ifStatement(
192226
t.unaryExpression('!', condition),
193-
t.blockStatement([
194-
t.throwStatement(
195-
t.callExpression(t.identifier('Error'), [devMessage])
196-
),
197-
])
227+
t.blockStatement([t.throwStatement(errorCall)])
198228
)
199229
);
200230
parentStatementPath.addComment(
@@ -219,6 +249,11 @@ module.exports = function(babel) {
219249
[t.numericLiteral(prodErrorId), ...errorMsgExpressions]
220250
);
221251

252+
const errorCall = t.callExpression(t.identifier('Error'), [
253+
t.conditionalExpression(DEV_EXPRESSION, devMessage, prodMessage),
254+
]);
255+
errorCall[SEEN_SYMBOL] = true;
256+
222257
// Outputs:
223258
// if (!condition) {
224259
// throw Error(
@@ -231,17 +266,7 @@ module.exports = function(babel) {
231266
t.ifStatement(
232267
t.unaryExpression('!', condition),
233268
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-
]),
269+
t.blockStatement([t.throwStatement(errorCall)]),
245270
])
246271
)
247272
);

0 commit comments

Comments
 (0)