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/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/__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..2c5263a8ff141 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.", @@ -397,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." } 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 6ebb68731f860..2baf4baa1c1a7 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -7,20 +7,114 @@ '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'); +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: // @@ -40,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') @@ -81,12 +175,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) { @@ -117,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;