From c499fe64485d865f482aa728ec855fa3ce050fc4 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 25 Sep 2021 13:03:14 -0400 Subject: [PATCH 1/4] Hoist error codes import to module scope When this code was written, the error codes map (`codes.json`) was created on-the-fly, so we had to lazily require from inside the visitor. Because `codes.json` is now checked into source, we can import it a single time in module scope. --- scripts/error-codes/transform-error-messages.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index 6ebb68731f860..ee01454dbd886 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -11,6 +11,10 @@ const evalToString = require('../shared/evalToString'); const invertObject = require('./invertObject'); const helperModuleImports = require('@babel/helper-module-imports'); +const errorMap = invertObject( + JSON.parse(fs.readFileSync(__dirname + '/codes.json', 'utf-8')) +); + module.exports = function(babel) { const t = babel.types; @@ -81,12 +85,6 @@ module.exports = function(babel) { return; } - // Avoid caching because we write it as we go. - const existingErrorMap = JSON.parse( - fs.readFileSync(__dirname + '/codes.json', 'utf-8') - ); - const errorMap = invertObject(existingErrorMap); - let prodErrorId = errorMap[errorMsgLiteral]; if (prodErrorId === undefined) { From 5286b8b65a9cbd50c4e9b50c9e09bc4f3fd275c2 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 25 Sep 2021 13:27:06 -0400 Subject: [PATCH 2/4] Minify error constructors in production We use a script to minify our error messages in production. Each message is assigned an error code, defined in `scripts/error-codes/codes.json`. Then our build script replaces the messages with a link to our error decoder page, e.g. https://reactjs.org/docs/error-decoder.html/?invariant=92 This enables us to write helpful error messages without increasing the bundle size. Right now, the script only works for `invariant` calls. It does not work if you throw an Error object. This is an old Facebookism that we don't really need, other than the fact that our error minification script relies on it. So, I've updated the script to minify error constructors, too: Input: Error(`A ${adj} message that contains ${noun}`); Output: Error(formatProdErrorMessage(ERR_CODE, adj, noun)); It only works for constructors that are literally named Error, though we could add support for other names, too. As a next step, I will add a lint rule to enforce that errors written this way must have a corresponding error code. --- packages/jest-react/src/JestReact.js | 12 +-- .../transform-error-messages.js.snap | 35 +++++++ .../__tests__/transform-error-messages.js | 57 +++++++++++ scripts/error-codes/codes.json | 2 - scripts/error-codes/extract-errors.js | 4 +- .../error-codes/transform-error-messages.js | 96 ++++++++++++++++++- scripts/print-warnings/print-warnings.js | 4 +- scripts/shared/__tests__/evalToString-test.js | 4 +- scripts/shared/evalToString.js | 40 +++++++- 9 files changed, 234 insertions(+), 20 deletions(-) diff --git a/packages/jest-react/src/JestReact.js b/packages/jest-react/src/JestReact.js index b4688f1ff980a..92f7e7cdb192d 100644 --- a/packages/jest-react/src/JestReact.js +++ b/packages/jest-react/src/JestReact.js @@ -7,7 +7,6 @@ import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols'; -import invariant from 'shared/invariant'; import isArray from 'shared/isArray'; export {act} from './internalAct'; @@ -31,11 +30,12 @@ function captureAssertion(fn) { function assertYieldsWereCleared(root) { const Scheduler = root._Scheduler; const actualYields = Scheduler.unstable_clearYields(); - invariant( - actualYields.length === 0, - 'Log of yielded values is not empty. ' + - 'Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.', - ); + if (actualYields.length !== 0) { + throw new Error( + 'Log of yielded values is not empty. ' + + 'Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.', + ); + } } export function unstable_toMatchRenderedOutput(root, expectedJSX) { 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 f93b31a659dd2..bfb80ab375562 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -1,5 +1,10 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`error transform handles escaped backticks in template string 1`] = ` +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; +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'; @@ -18,6 +23,21 @@ if (!condition) { }" `; +exports[`error transform should not touch other calls or new expressions 1`] = ` +"new NotAnError(); +NotAnError();" +`; + +exports[`error transform should replace error constructors (no new) 1`] = ` +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; +Error(__DEV__ ? 'Do not override existing functions.' : _formatProdErrorMessage(16));" +`; + +exports[`error transform should replace error constructors 1`] = ` +"import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; +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'; @@ -29,6 +49,21 @@ if (!condition) { }" `; +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));" +`; + +exports[`error transform should support interpolating arguments with concatenation 1`] = ` +"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 interpolating arguments with template strings 1`] = ` +"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'; diff --git a/scripts/error-codes/__tests__/transform-error-messages.js b/scripts/error-codes/__tests__/transform-error-messages.js index 4da00ac656b95..2bca7366321e2 100644 --- a/scripts/error-codes/__tests__/transform-error-messages.js +++ b/scripts/error-codes/__tests__/transform-error-messages.js @@ -93,4 +93,61 @@ invariant(condition, 'Do not override existing functions.'); ) ).toMatchSnapshot(); }); + + it('should replace error constructors', () => { + expect( + transform(` +new Error('Do not override existing functions.'); +`) + ).toMatchSnapshot(); + }); + + it('should replace error constructors (no new)', () => { + expect( + transform(` +Error('Do not override existing functions.'); +`) + ).toMatchSnapshot(); + }); + + it('should not touch other calls or new expressions', () => { + expect( + transform(` +new NotAnError(); +NotAnError(); +`) + ).toMatchSnapshot(); + }); + + it('should support interpolating arguments with template strings', () => { + expect( + transform(` +new Error(\`Expected \${foo} target to be an array; got \${bar}\`); +`) + ).toMatchSnapshot(); + }); + + it('should support interpolating arguments with concatenation', () => { + expect( + transform(` +new Error('Expected ' + foo + ' target to be an array; got ' + bar); +`) + ).toMatchSnapshot(); + }); + + it('should support error constructors with concatenated messages', () => { + expect( + transform(` +new Error(\`Expected \${foo} target to \` + \`be an array; got \${bar}\`); +`) + ).toMatchSnapshot(); + }); + + it('handles escaped backticks in template string', () => { + expect( + transform(` +new Error(\`Expected \\\`\$\{listener\}\\\` listener to be a function, instead got a value of \\\`\$\{type\}\\\` type.\`); +`) + ).toMatchSnapshot(); + }); }); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 3206efa41349c..293ad42c151a9 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -287,12 +287,10 @@ "288": "It is not supported to run the profiling version of a renderer (for example, `react-dom/profiling`) without also replacing the `schedule/tracing` module with `schedule/tracing-profiling`. Your bundler might have a setting for aliasing both modules. Learn more at https://reactjs.org/link/profiling", "289": "Function components cannot have refs.", "290": "Element ref was specified as a string (%s) but no owner was set. This could happen for one of the following reasons:\n1. You may be adding a ref to a function component\n2. You may be adding a ref to a component that was not created inside a component's render method\n3. You have multiple copies of React loaded\nSee https://reactjs.org/link/refs-must-have-owner for more information.", - "291": "Log of yielded values is not empty. Call expect(Scheduler).toHaveYielded(...) first.", "292": "The matcher `toHaveYielded` expects an instance of React Test Renderer.\n\nTry: expect(Scheduler).toHaveYielded(expectedYields)", "293": "Context can only be read while React is rendering, e.g. inside the render method or getDerivedStateFromProps.", "294": "ReactDOMServer does not yet support Suspense.", "295": "ReactDOMServer does not yet support lazy-loaded components.", - "296": "Log of yielded values is not empty. Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.", "297": "The matcher `unstable_toHaveYielded` expects an instance of React Test Renderer.\n\nTry: expect(ReactTestRenderer).unstable_toHaveYielded(expectedYields)", "298": "Hooks can only be called inside the body of a function component.", "299": "createRoot(...): Target container is not a DOM element.", diff --git a/scripts/error-codes/extract-errors.js b/scripts/error-codes/extract-errors.js index 58823cbe16873..d60ffe308cdbe 100644 --- a/scripts/error-codes/extract-errors.js +++ b/scripts/error-codes/extract-errors.js @@ -10,7 +10,7 @@ const parser = require('@babel/parser'); const fs = require('fs'); const path = require('path'); const traverse = require('@babel/traverse').default; -const evalToString = require('../shared/evalToString'); +const {evalStringConcat} = require('../shared/evalToString'); const invertObject = require('./invertObject'); const babylonOptions = { @@ -75,7 +75,7 @@ module.exports = function(opts) { // error messages can be concatenated (`+`) at runtime, so here's a // trivial partial evaluator that interprets the literal value - const errorMsgLiteral = evalToString(node.arguments[1]); + const errorMsgLiteral = evalStringConcat(node.arguments[1]); addToErrorMap(errorMsgLiteral); } }, diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index ee01454dbd886..2baf4baa1c1a7 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -7,7 +7,10 @@ 'use strict'; const fs = require('fs'); -const evalToString = require('../shared/evalToString'); +const { + evalStringConcat, + evalStringAndTemplateConcat, +} = require('../shared/evalToString'); const invertObject = require('./invertObject'); const helperModuleImports = require('@babel/helper-module-imports'); @@ -15,16 +18,103 @@ const errorMap = invertObject( JSON.parse(fs.readFileSync(__dirname + '/codes.json', 'utf-8')) ); +const SEEN_SYMBOL = Symbol('transform-error-messages.seen'); + module.exports = function(babel) { const t = babel.types; + // TODO: Instead of outputting __DEV__ conditions, only apply this transform + // in production. const DEV_EXPRESSION = t.identifier('__DEV__'); + function CallOrNewExpression(path, file) { + // Turns this code: + // + // new Error(`A ${adj} message that contains ${noun}`); + // + // or this code (no constructor): + // + // Error(`A ${adj} message that contains ${noun}`); + // + // into this: + // + // Error( + // __DEV__ + // ? `A ${adj} message that contains ${noun}` + // : formatProdErrorMessage(ERR_CODE, adj, noun) + // ); + const node = path.node; + if (node[SEEN_SYMBOL]) { + return; + } + node[SEEN_SYMBOL] = true; + + const errorMsgNode = node.arguments[0]; + if (errorMsgNode === undefined) { + return; + } + + const errorMsgExpressions = []; + const errorMsgLiteral = evalStringAndTemplateConcat( + errorMsgNode, + errorMsgExpressions + ); + + let prodErrorId = errorMap[errorMsgLiteral]; + if (prodErrorId === undefined) { + // There is no error code for this message. We use a lint rule to + // enforce that messages can be minified, so assume this is + // intentional and exit gracefully. + 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, + ]); + + // Outputs: + // Error( + // __DEV__ + // ? `A ${adj} message that contains ${noun}` + // : formatProdErrorMessage(ERR_CODE, adj, noun) + // ); + path.replaceWith(t.callExpression(t.identifier('Error'), [prodMessage])); + path.replaceWith( + t.callExpression(t.identifier('Error'), [ + t.conditionalExpression(DEV_EXPRESSION, errorMsgNode, prodMessage), + ]) + ); + } + return { visitor: { + NewExpression(path, file) { + const noMinify = file.opts.noMinify; + if (!noMinify && path.get('callee').isIdentifier({name: 'Error'})) { + CallOrNewExpression(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); + return; + } + if (path.get('callee').isIdentifier({name: 'invariant'})) { // Turns this code: // @@ -44,7 +134,7 @@ module.exports = function(babel) { // string) that references a verbose error message. The mapping is // stored in `scripts/error-codes/codes.json`. const condition = node.arguments[0]; - const errorMsgLiteral = evalToString(node.arguments[1]); + const errorMsgLiteral = evalStringConcat(node.arguments[1]); const errorMsgExpressions = Array.from(node.arguments.slice(2)); const errorMsgQuasis = errorMsgLiteral .split('%s') @@ -115,7 +205,7 @@ module.exports = function(babel) { } prodErrorId = parseInt(prodErrorId, 10); - // Import ReactErrorProd + // Import formatProdErrorMessage const formatProdErrorMessageIdentifier = helperModuleImports.addDefault( path, 'shared/formatProdErrorMessage', diff --git a/scripts/print-warnings/print-warnings.js b/scripts/print-warnings/print-warnings.js index 9bdd6543d2a3d..8e23dd880e92b 100644 --- a/scripts/print-warnings/print-warnings.js +++ b/scripts/print-warnings/print-warnings.js @@ -12,7 +12,7 @@ const through = require('through2'); const traverse = require('@babel/traverse').default; const gs = require('glob-stream'); -const evalToString = require('../shared/evalToString'); +const {evalStringConcat} = require('../shared/evalToString'); const parserOptions = { sourceType: 'module', @@ -64,7 +64,7 @@ function transform(file, enc, cb) { // warning messages can be concatenated (`+`) at runtime, so here's // a trivial partial evaluator that interprets the literal value try { - const warningMsgLiteral = evalToString(node.arguments[0]); + const warningMsgLiteral = evalStringConcat(node.arguments[0]); warnings.add(JSON.stringify(warningMsgLiteral)); } catch (error) { console.error( diff --git a/scripts/shared/__tests__/evalToString-test.js b/scripts/shared/__tests__/evalToString-test.js index 3f6c3adbb372a..bc0cb600d57e4 100644 --- a/scripts/shared/__tests__/evalToString-test.js +++ b/scripts/shared/__tests__/evalToString-test.js @@ -6,12 +6,12 @@ */ 'use strict'; -const evalToString = require('../evalToString'); +const {evalStringConcat} = require('../evalToString'); const parser = require('@babel/parser'); const parse = source => parser.parse(`(${source});`).program.body[0].expression; // quick way to get an exp node -const parseAndEval = source => evalToString(parse(source)); +const parseAndEval = source => evalStringConcat(parse(source)); describe('evalToString', () => { it('should support StringLiteral', () => { diff --git a/scripts/shared/evalToString.js b/scripts/shared/evalToString.js index aad199da5d75b..318925cc0969e 100644 --- a/scripts/shared/evalToString.js +++ b/scripts/shared/evalToString.js @@ -8,7 +8,7 @@ */ 'use strict'; -function evalToString(ast /* : Object */) /* : string */ { +function evalStringConcat(ast /* : Object */) /* : string */ { switch (ast.type) { case 'StringLiteral': case 'Literal': // ESLint @@ -17,10 +17,44 @@ function evalToString(ast /* : Object */) /* : string */ { if (ast.operator !== '+') { throw new Error('Unsupported binary operator ' + ast.operator); } - return evalToString(ast.left) + evalToString(ast.right); + return evalStringConcat(ast.left) + evalStringConcat(ast.right); default: throw new Error('Unsupported type ' + ast.type); } } +exports.evalStringConcat = evalStringConcat; -module.exports = evalToString; +function evalStringAndTemplateConcat( + ast /* : Object */, + args /* : Array */ +) /* : string */ { + switch (ast.type) { + case 'StringLiteral': + return ast.value; + case 'BinaryExpression': // `+` + if (ast.operator !== '+') { + throw new Error('Unsupported binary operator ' + ast.operator); + } + return ( + evalStringAndTemplateConcat(ast.left, args) + + evalStringAndTemplateConcat(ast.right, args) + ); + case 'TemplateLiteral': { + let elements = []; + for (let i = 0; i < ast.quasis.length; i++) { + const elementNode = ast.quasis[i]; + if (elementNode.type !== 'TemplateElement') { + throw new Error('Unsupported type ' + ast.type); + } + elements.push(elementNode.value.cooked); + } + args.push(...ast.expressions); + return elements.join('%s'); + } + default: + // Anything that's not a string is interpreted as an argument. + args.push(ast); + return '%s'; + } +} +exports.evalStringAndTemplateConcat = evalStringAndTemplateConcat; From 2bcea20401003a0959cf2e5fa6074bfbc0e56da9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 25 Sep 2021 14:30:30 -0400 Subject: [PATCH 3/4] Minify "no fallback UI specified" error in prod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This error message wasn't being minified because it doesn't use invariant. The reason it didn't use invariant is because this particular error is created without begin thrown — it doesn't need to be thrown because it's located inside the error handling part of the runtime. Now that the error minification script supports Error constructors, we can minify it by assigning it a production error code in `scripts/error-codes/codes.json`. To support the use of Error constructors more generally, I will add a lint rule that enforces each message has a corresponding error code. --- packages/react-reconciler/src/ReactFiberThrow.new.js | 3 ++- packages/react-reconciler/src/ReactFiberThrow.old.js | 3 ++- scripts/error-codes/codes.json | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 78e88f125a1aa..85728d96bb730 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -474,7 +474,8 @@ function throwException( return; } else { // No boundary was found. Fallthrough to error mode. - // TODO: Use invariant so the message is stripped in prod? + // TODO: We should never call getComponentNameFromFiber in production. + // Log a warning or something to prevent us from accidentally bundling it. value = new Error( (getComponentNameFromFiber(sourceFiber) || 'A React component') + ' suspended while rendering, but no fallback UI was specified.\n' + diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index d2d39793a3bc0..27b0719ba8532 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -474,7 +474,8 @@ function throwException( return; } else { // No boundary was found. Fallthrough to error mode. - // TODO: Use invariant so the message is stripped in prod? + // TODO: We should never call getComponentNameFromFiber in production. + // Log a warning or something to prevent us from accidentally bundling it. value = new Error( (getComponentNameFromFiber(sourceFiber) || 'A React component') + ' suspended while rendering, but no fallback UI was specified.\n' + diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 293ad42c151a9..2c5263a8ff141 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -395,5 +395,6 @@ "406": "act(...) is not supported in production builds of React.", "407": "Missing getServerSnapshot, which is required for server-rendered content. Will revert to client rendering.", "408": "Missing getServerSnapshot, which is required for server-rendered content.", - "409": "Cannot update an unmounted root." + "409": "Cannot update an unmounted root.", + "410": "%s suspended while rendering, but no fallback UI was specified.\n\nAdd a component higher in the tree to provide a loading indicator or placeholder to display." } From 7d40b8a9373d9af4b374e83bff7ad6392aa92233 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 25 Sep 2021 16:47:05 -0400 Subject: [PATCH 4/4] Lint rule to detect unminified errors Adds a lint rule that detects when an Error constructor is used without a corresponding production error code. We already have this for `invariant`, but not for regular errors, i.e. `throw new Error(msg)`. There's also nothing that enforces the use of `invariant` besides convention. There are some packages where we don't care to minify errors. These are packages that run in environments where bundle size is not a concern, like react-pg. I added an override in the ESLint config to ignore these. --- .eslintrc.js | 35 ++++++++ packages/react-art/src/ReactARTHostConfig.js | 10 +-- packages/react-cache/src/ReactCacheOld.js | 2 + .../react-client/src/ReactFlightClient.js | 1 + .../ReactFlightClientHostConfigNoStream.js | 3 + packages/react-fetch/src/ReactFetchBrowser.js | 1 + packages/react-fetch/src/ReactFetchNode.js | 1 + .../src/ReactFiberBeginWork.new.js | 4 + .../src/ReactFiberBeginWork.old.js | 4 + .../src/ReactFiberHooks.new.js | 1 + .../src/ReactFiberHooks.old.js | 1 + .../src/ReactFiberHotReloading.new.js | 2 + .../src/ReactFiberHotReloading.old.js | 2 + packages/react/unstable-shared-subset.js | 1 + packages/scheduler/src/forks/SchedulerMock.js | 1 + packages/shared/checkPropTypes.js | 1 + packages/shared/invariant.js | 1 + packages/shared/invokeGuardedCallbackImpl.js | 2 + scripts/error-codes/codes.json | 8 +- .../prod-error-codes-test.internal.js | 77 ++++++++++++++++++ scripts/eslint-rules/index.js | 1 + scripts/eslint-rules/prod-error-codes.js | 79 +++++++++++++++++++ 22 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 scripts/eslint-rules/__tests__/prod-error-codes-test.internal.js create mode 100644 scripts/eslint-rules/prod-error-codes.js diff --git a/.eslintrc.js b/.eslintrc.js index 23d5ab76c32bc..d464e83b453aa 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -127,6 +127,41 @@ module.exports = { }, overrides: [ + { + // By default, anything error message that appears the packages directory + // must have a corresponding error code. The exceptions are defined + // in the next override entry. + files: ['packages/**/*.js'], + rules: { + 'react-internal/prod-error-codes': ERROR, + }, + }, + { + // These are files where it's OK to have unminified error messages. These + // are environments where bundle size isn't a concern, like tests + // or Node. + files: [ + 'packages/react-dom/src/test-utils/**/*.js', + 'packages/react-devtools-shared/**/*.js', + 'packages/react-noop-renderer/**/*.js', + 'packages/react-pg/**/*.js', + 'packages/react-fs/**/*.js', + 'packages/react-refresh/**/*.js', + 'packages/react-server-dom-webpack/**/*.js', + 'packages/react-test-renderer/**/*.js', + 'packages/react-debug-tools/**/*.js', + 'packages/react-devtools-extensions/**/*.js', + 'packages/react-devtools-scheduling-profiler/**/*.js', + 'packages/react-native-renderer/**/*.js', + 'packages/eslint-plugin-react-hooks/**/*.js', + 'packages/jest-react/**/*.js', + 'packages/**/__tests__/*.js', + 'packages/**/npm/*.js', + ], + rules: { + 'react-internal/prod-error-codes': OFF, + }, + }, { // We apply these settings to files that we ship through npm. // They must be ES5. diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index f9971b5bb0360..baceba93641ec 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -433,25 +433,25 @@ export function clearContainer(container) { } export function getInstanceFromNode(node) { - throw new Error('Not yet implemented.'); + throw new Error('Not implemented.'); } export function isOpaqueHydratingObject(value: mixed): boolean { - throw new Error('Not yet implemented'); + throw new Error('Not implemented.'); } export function makeOpaqueHydratingObject( attemptToReadValue: () => void, ): OpaqueIDType { - throw new Error('Not yet implemented.'); + throw new Error('Not implemented.'); } export function makeClientId(): OpaqueIDType { - throw new Error('Not yet implemented'); + throw new Error('Not implemented.'); } export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType { - throw new Error('Not yet implemented'); + throw new Error('Not implemented.'); } export function beforeActiveInstanceBlur(internalInstanceHandle: Object) { diff --git a/packages/react-cache/src/ReactCacheOld.js b/packages/react-cache/src/ReactCacheOld.js index 77768dfcf2163..f8b33434b7c76 100644 --- a/packages/react-cache/src/ReactCacheOld.js +++ b/packages/react-cache/src/ReactCacheOld.js @@ -49,6 +49,8 @@ const ReactCurrentDispatcher = function readContext(Context) { const dispatcher = ReactCurrentDispatcher.current; if (dispatcher === null) { + // This wasn't being minified but we're going to retire this package anyway. + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'react-cache: read and preload may only be called from within a ' + "component's render. They are not supported in event handlers or " + diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 983e3edc93802..9d5ac3680a8a0 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -397,6 +397,7 @@ export function resolveError( message: string, stack: string, ): void { + // eslint-disable-next-line react-internal/prod-error-codes const error = new Error(message); error.stack = stack; const chunks = response._chunks; diff --git a/packages/react-client/src/ReactFlightClientHostConfigNoStream.js b/packages/react-client/src/ReactFlightClientHostConfigNoStream.js index 17f29a9f26c50..8c453832bd3b2 100644 --- a/packages/react-client/src/ReactFlightClientHostConfigNoStream.js +++ b/packages/react-client/src/ReactFlightClientHostConfigNoStream.js @@ -12,6 +12,7 @@ export type StringDecoder = void; export const supportsBinaryStreams = false; export function createStringDecoder(): void { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Should never be called'); } @@ -19,6 +20,7 @@ export function readPartialStringChunk( decoder: StringDecoder, buffer: Uint8Array, ): string { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Should never be called'); } @@ -26,5 +28,6 @@ export function readFinalStringChunk( decoder: StringDecoder, buffer: Uint8Array, ): string { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Should never be called'); } diff --git a/packages/react-fetch/src/ReactFetchBrowser.js b/packages/react-fetch/src/ReactFetchBrowser.js index fbf5b50b2f29e..41df29c8afcc2 100644 --- a/packages/react-fetch/src/ReactFetchBrowser.js +++ b/packages/react-fetch/src/ReactFetchBrowser.js @@ -129,6 +129,7 @@ function preloadRecord(url: string, options: mixed): Record { if (options.method || options.body || options.signal) { // TODO: wire up our own cancellation mechanism. // TODO: figure out what to do with POST. + // eslint-disable-next-line react-internal/prod-error-codes throw Error('Unsupported option'); } } diff --git a/packages/react-fetch/src/ReactFetchNode.js b/packages/react-fetch/src/ReactFetchNode.js index 727c43abec415..2b1ef5f200cb1 100644 --- a/packages/react-fetch/src/ReactFetchNode.js +++ b/packages/react-fetch/src/ReactFetchNode.js @@ -177,6 +177,7 @@ function preloadRecord(url: string, options: mixed): Record { if (options.method || options.body || options.signal) { // TODO: wire up our own cancellation mechanism. // TODO: figure out what to do with POST. + // eslint-disable-next-line react-internal/prod-error-codes throw Error('Unsupported option'); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 6ad63ae763c2a..65d9c2e96c9e4 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -1050,6 +1050,7 @@ function updateClassComponent( case true: { workInProgress.flags |= DidCapture; workInProgress.flags |= ShouldCapture; + // eslint-disable-next-line react-internal/prod-error-codes const error = new Error('Simulated error coming from DevTools'); const lane = pickArbitraryLane(renderLanes); workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); @@ -3317,6 +3318,7 @@ function remountFiber( if (__DEV__) { const returnFiber = oldWorkInProgress.return; if (returnFiber === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Cannot swap the root fiber.'); } @@ -3337,11 +3339,13 @@ function remountFiber( } else { let prevSibling = returnFiber.child; if (prevSibling === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Expected parent to have a child.'); } while (prevSibling.sibling !== oldWorkInProgress) { prevSibling = prevSibling.sibling; if (prevSibling === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Expected to find the previous sibling.'); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index ffa753593e1a4..77622d0e4f06e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -1050,6 +1050,7 @@ function updateClassComponent( case true: { workInProgress.flags |= DidCapture; workInProgress.flags |= ShouldCapture; + // eslint-disable-next-line react-internal/prod-error-codes const error = new Error('Simulated error coming from DevTools'); const lane = pickArbitraryLane(renderLanes); workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); @@ -3317,6 +3318,7 @@ function remountFiber( if (__DEV__) { const returnFiber = oldWorkInProgress.return; if (returnFiber === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Cannot swap the root fiber.'); } @@ -3337,11 +3339,13 @@ function remountFiber( } else { let prevSibling = returnFiber.child; if (prevSibling === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Expected parent to have a child.'); } while (prevSibling.sibling !== oldWorkInProgress) { prevSibling = prevSibling.sibling; if (prevSibling === null) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error('Expected to find the previous sibling.'); } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index d6123454972ac..b64cd0294b303 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -1532,6 +1532,7 @@ function pushEffect(tag, create, destroy, deps) { let stackContainsErrorMessage: boolean | null = null; function getCallerStackFrame(): string { + // eslint-disable-next-line react-internal/prod-error-codes const stackFrames = new Error('Error message').stack.split('\n'); // Some browsers (e.g. Chrome) include the error message in the stack diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index d4dacbc8833ce..87efa6ed2158d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -1532,6 +1532,7 @@ function pushEffect(tag, create, destroy, deps) { let stackContainsErrorMessage: boolean | null = null; function getCallerStackFrame(): string { + // eslint-disable-next-line react-internal/prod-error-codes const stackFrames = new Error('Error message').stack.split('\n'); // Some browsers (e.g. Chrome) include the error message in the stack diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.new.js b/packages/react-reconciler/src/ReactFiberHotReloading.new.js index 4c9eaf010125c..0867ffeb78439 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.new.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.new.js @@ -7,6 +7,8 @@ * @flow */ +/* eslint-disable react-internal/prod-error-codes */ + import type {ReactElement} from 'shared/ReactElementType'; import type {Fiber} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.old.js b/packages/react-reconciler/src/ReactFiberHotReloading.old.js index ee0616fae79c0..4a5f53d3be731 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.old.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.old.js @@ -7,6 +7,8 @@ * @flow */ +/* eslint-disable react-internal/prod-error-codes */ + import type {ReactElement} from 'shared/ReactElementType'; import type {Fiber} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; diff --git a/packages/react/unstable-shared-subset.js b/packages/react/unstable-shared-subset.js index e2ae4ad2f62d6..a91fe77a1dc0d 100644 --- a/packages/react/unstable-shared-subset.js +++ b/packages/react/unstable-shared-subset.js @@ -7,6 +7,7 @@ * @flow */ +// eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'This entry point is not yet supported outside of experimental channels', ); diff --git a/packages/scheduler/src/forks/SchedulerMock.js b/packages/scheduler/src/forks/SchedulerMock.js index 21c258a6b528c..5f7c8dc8e83aa 100644 --- a/packages/scheduler/src/forks/SchedulerMock.js +++ b/packages/scheduler/src/forks/SchedulerMock.js @@ -7,6 +7,7 @@ */ /* eslint-disable no-var */ +/* eslint-disable react-internal/prod-error-codes */ import { enableSchedulerDebugging, diff --git a/packages/shared/checkPropTypes.js b/packages/shared/checkPropTypes.js index 1558a407c3888..5aee8fede8330 100644 --- a/packages/shared/checkPropTypes.js +++ b/packages/shared/checkPropTypes.js @@ -52,6 +52,7 @@ export default function checkPropTypes( // This is intentionally an invariant that gets caught. It's the same // behavior as without this statement except with a better message. if (typeof typeSpecs[typeSpecName] !== 'function') { + // eslint-disable-next-line react-internal/prod-error-codes const err = Error( (componentName || 'React class') + ': ' + diff --git a/packages/shared/invariant.js b/packages/shared/invariant.js index 747961671f1ea..68a1928c02d38 100644 --- a/packages/shared/invariant.js +++ b/packages/shared/invariant.js @@ -18,6 +18,7 @@ */ export default function invariant(condition, format, a, b, c, d, e, f) { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'Internal React error: invariant() is meant to be replaced at compile ' + 'time. There is no runtime version.', diff --git a/packages/shared/invokeGuardedCallbackImpl.js b/packages/shared/invokeGuardedCallbackImpl.js index 215a8ae91c513..e40fdb2746ce9 100644 --- a/packages/shared/invokeGuardedCallbackImpl.js +++ b/packages/shared/invokeGuardedCallbackImpl.js @@ -201,6 +201,7 @@ if (__DEV__) { if (didCall && didError) { if (!didSetError) { // The callback errored, but the error event never fired. + // eslint-disable-next-line react-internal/prod-error-codes error = new Error( 'An error was thrown inside one of your components, but React ' + "doesn't know what it was. This is likely due to browser " + @@ -212,6 +213,7 @@ if (__DEV__) { 'actually an issue with React, please file an issue.', ); } else if (isCrossOriginError) { + // eslint-disable-next-line react-internal/prod-error-codes error = new Error( "A cross-origin error was thrown. React doesn't have access to " + 'the actual error object in development. ' + diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 2c5263a8ff141..1ae480901b970 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -396,5 +396,11 @@ "407": "Missing getServerSnapshot, which is required for server-rendered content. Will revert to client rendering.", "408": "Missing getServerSnapshot, which is required for server-rendered content.", "409": "Cannot update an unmounted root.", - "410": "%s suspended while rendering, but no fallback UI was specified.\n\nAdd a component higher in the tree to provide a loading indicator or placeholder to display." + "410": "%s suspended while rendering, but no fallback UI was specified.\n\nAdd a component higher in the tree to provide a loading indicator or placeholder to display.", + "411": "%s suspended while rendering, but no fallback UI was specified.\n\nAdd a component higher in the tree to provide a loading indicator or placeholder to display.", + "412": "Connection closed.", + "413": "Expected finished root and lanes to be set. This is a bug in React.", + "414": "Did not expect this call in production. This is a bug in React. Please file an issue.", + "415": "Error parsing the data. It's probably an error code or network corruption.", + "416": "This environment don't support binary chunks." } diff --git a/scripts/eslint-rules/__tests__/prod-error-codes-test.internal.js b/scripts/eslint-rules/__tests__/prod-error-codes-test.internal.js new file mode 100644 index 0000000000000..674d952e9582c --- /dev/null +++ b/scripts/eslint-rules/__tests__/prod-error-codes-test.internal.js @@ -0,0 +1,77 @@ +/** + * 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. + * + * @emails react-core + */ + +'use strict'; + +const rule = require('../prod-error-codes'); +const {RuleTester} = require('eslint'); +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2017, + }, +}); + +ruleTester.run('eslint-rules/prod-error-codes', rule, { + valid: [ + 'arbitraryFunction(a, b)', + 'Error(`Expected ${foo} target to be an array; got ${bar}`)', + "Error('Expected ' + foo + ' target to be an array; got ' + bar)", + 'Error(`Expected ${foo} target to ` + `be an array; got ${bar}`)', + ], + invalid: [ + { + code: "Error('Not in error map')", + errors: [ + { + message: + 'Error message does not have a corresponding production error ' + + 'code. Add the following message to codes.json so it can be stripped from ' + + 'the production builds:\n\n' + + 'Not in error map', + }, + ], + }, + { + code: "Error('Not in ' + 'error map')", + errors: [ + { + message: + 'Error message does not have a corresponding production error ' + + 'code. Add the following message to codes.json so it can be stripped from ' + + 'the production builds:\n\n' + + 'Not in error map', + }, + ], + }, + { + code: 'Error(`Not in ` + `error map`)', + errors: [ + { + message: + 'Error message does not have a corresponding production error ' + + 'code. Add the following message to codes.json so it can be stripped from ' + + 'the production builds:\n\n' + + 'Not in error map', + }, + ], + }, + { + code: "Error(`Not in ${'error'} map`)", + errors: [ + { + message: + 'Error message does not have a corresponding production error ' + + 'code. Add the following message to codes.json so it can be stripped from ' + + 'the production builds:\n\n' + + 'Not in %s map', + }, + ], + }, + ], +}); diff --git a/scripts/eslint-rules/index.js b/scripts/eslint-rules/index.js index 0e86334d26331..7dfac457ec062 100644 --- a/scripts/eslint-rules/index.js +++ b/scripts/eslint-rules/index.js @@ -6,6 +6,7 @@ module.exports = { 'no-to-warn-dev-within-to-throw': require('./no-to-warn-dev-within-to-throw'), 'warning-args': require('./warning-args'), 'invariant-args': require('./invariant-args'), + 'prod-error-codes': require('./prod-error-codes'), 'no-production-logging': require('./no-production-logging'), 'no-cross-fork-imports': require('./no-cross-fork-imports'), 'no-cross-fork-types': require('./no-cross-fork-types'), diff --git a/scripts/eslint-rules/prod-error-codes.js b/scripts/eslint-rules/prod-error-codes.js new file mode 100644 index 0000000000000..2c9815fb9ab12 --- /dev/null +++ b/scripts/eslint-rules/prod-error-codes.js @@ -0,0 +1,79 @@ +/** + * 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. + * + * @emails react-core + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const errorMap = JSON.parse( + fs.readFileSync(path.resolve(__dirname, '../error-codes/codes.json')) +); +const errorMessages = new Set(); +Object.keys(errorMap).forEach(key => errorMessages.add(errorMap[key])); + +function nodeToErrorTemplate(node) { + if (node.type === 'Literal' && typeof node.value === 'string') { + return node.value; + } else if (node.type === 'BinaryExpression' && node.operator === '+') { + const l = nodeToErrorTemplate(node.left); + const r = nodeToErrorTemplate(node.right); + return l + r; + } else if (node.type === 'TemplateLiteral') { + let elements = []; + for (let i = 0; i < node.quasis.length; i++) { + const elementNode = node.quasis[i]; + if (elementNode.type !== 'TemplateElement') { + throw new Error('Unsupported type ' + node.type); + } + elements.push(elementNode.value.cooked); + } + return elements.join('%s'); + } else { + return '%s'; + } +} + +module.exports = { + meta: { + schema: [], + }, + create(context) { + function ErrorCallExpression(node) { + const errorMessageNode = node.arguments[0]; + if (errorMessageNode === undefined) { + return; + } + const errorMessage = nodeToErrorTemplate(errorMessageNode); + if (errorMessages.has(errorMessage)) { + return; + } + context.report({ + node, + message: + 'Error message does not have a corresponding production error code. Add ' + + 'the following message to codes.json so it can be stripped ' + + 'from the production builds:\n\n' + + errorMessage, + }); + } + + return { + NewExpression(node) { + if (node.callee.type === 'Identifier' && node.callee.name === 'Error') { + ErrorCallExpression(node); + } + }, + CallExpression(node) { + if (node.callee.type === 'Identifier' && node.callee.name === 'Error') { + ErrorCallExpression(node); + } + }, + }; + }, +};