Follow-up improvements to error code extraction infra

.circleci/config.yml

@@ -209,7 +209,20 @@ jobs:
       - run: yarn workspaces info | head -n -1 > workspace_info.txt
       - *restore_node_modules
       - run: yarn lint-build
-      - run: scripts/circleci/check_minified_errors.sh
+
+  check_error_codes:
+    docker: *docker
+    environment: *environment
+    steps:
+      - checkout
+      - attach_workspace: *attach_workspace
+      - run: yarn workspaces info | head -n -1 > workspace_info.txt
+      - *restore_node_modules
+      - run:
+          name: Search build artifacts for unminified errors
+          command: |
+            yarn extract-errors
+            git diff || (echo "Found unminified errors. Either update the error codes map or disable error minification for the affected build, if appropriate." && false)
 
   yarn_test:
     docker: *docker
@@ -414,6 +427,9 @@ workflows:
       - yarn_lint_build:
           requires:
             - yarn_build_combined
+      - check_error_codes:
+          requires:
+            - yarn_build_combined
       - RELEASE_CHANNEL_stable_yarn_test_dom_fixtures:
           requires:
             - yarn_build_combined

package.json

@@ -114,7 +114,7 @@
     "linc": "node ./scripts/tasks/linc.js",
     "lint": "node ./scripts/tasks/eslint.js",
     "lint-build": "node ./scripts/rollup/validate/index.js",
-    "extract-errors": "yarn build --type=dev --extract-errors",
+    "extract-errors": "node scripts/error-codes/extract-errors.js",
     "postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js && node ./scripts/yarn/downloadReactIsForPrettyFormat.js",
     "debug-test": "yarn test --deprecated 'yarn test --debug'",
     "test": "node ./scripts/jest/jest-cli.js",

packages/shared/__tests__/ReactError-test.internal.js

@@ -37,6 +37,7 @@ describe('ReactError', () => {
   });
 
   // @gate build === "production"
+  // @gate !source
   it('should error with minified error code', () => {
     expect(() => ReactDOM.render('Hi', null)).toThrowError(
       'Minified React error #200; visit ' +

scripts/error-codes/README.md

@@ -9,7 +9,10 @@ provide a better debugging support in production. Check out the blog post
   the file will never be changed/removed.
 - [`extract-errors.js`](https://github.com/facebook/react/blob/main/scripts/error-codes/extract-errors.js)
   is an node script that traverses our codebase and updates `codes.json`. You
-  can test it by running `yarn extract-errors`.
+  can test it by running `yarn extract-errors`. It works by crawling the build
+  artifacts directory, so you need to have either run the build script or
+  downloaded pre-built artifacts (e.g. with `yarn download build`). It works
+  with partial builds, too.
 - [`transform-error-messages`](https://github.com/facebook/react/blob/main/scripts/error-codes/transform-error-messages.js)
   is a Babel pass that rewrites error messages to IDs for a production
   (minified) build.

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

@@ -2,94 +2,47 @@
 
 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));"
+Error(_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 not touch other calls or new expressions 1`] = `
+"new NotAnError();
+NotAnError();"
 `;
 
-exports[`error transform should handle escaped characters 1`] = `
-"import invariant from 'shared/invariant';
+exports[`error transform should output FIXME for errors that don't have a matching error code 1`] = `
+"/*! FIXME (minify-errors-in-prod): Unminified error message in production build!*/
 
-/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/
-if (!condition) {
-  throw Error(\\"What's up?\\");
-}"
+/*! <expected-error-format>\\"This is not a real error message.\\"</expected-error-format>*/
+Error('This is not a real error message.');"
 `;
 
-exports[`error transform should not touch other calls or new expressions 1`] = `
-"new NotAnError();
-NotAnError();"
+exports[`error transform should output FIXME for errors that don't have a matching error code, unless opted out with a comment 1`] = `
+"// eslint-disable-next-line react-internal/prod-error-codes
+Error('This is not a real error message.');"
 `;
 
 exports[`error transform should replace error constructors (no new) 1`] = `
 "import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\";
-Error(__DEV__ ? 'Do not override existing functions.' : _formatProdErrorMessage(16));"
+Error(_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';
-
-if (!condition) {
-  {
-    throw Error(__DEV__ ? \\"Do not override existing functions.\\" : _formatProdErrorMessage(16));
-  }
-}"
+Error(_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));"
+Error(_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));"
+Error(_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';
-
-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.\\");
-}"
+Error(_formatProdErrorMessage(7, foo, bar));"
 `;

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

@@ -28,87 +28,46 @@ 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', () => {
+  it('should replace error constructors', () => {
     expect(
       transform(`
-import invariant from 'shared/invariant';
-invariant(condition, 'This is not a real error message.');
+new Error('Do not override existing functions.');
 `)
     ).toMatchSnapshot();
   });
 
-  it('should handle escaped characters', () => {
+  it('should replace error constructors (no new)', () => {
     expect(
       transform(`
-import invariant from 'shared/invariant';
-invariant(condition, 'What\\'s up?');
+Error('Do not override existing functions.');
 `)
     ).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', () => {
+  it("should output FIXME for errors that don't have a matching error code", () => {
     expect(
       transform(`
-new Error('Do not override existing functions.');
+Error('This is not a real error message.');
 `)
     ).toMatchSnapshot();
   });
 
-  it('should replace error constructors (no new)', () => {
-    expect(
-      transform(`
-Error('Do not override existing functions.');
+  it(
+    "should output FIXME for errors that don't have a matching error " +
+      'code, unless opted out with a comment',
+    () => {
+      // TODO: Since this only detects one of many ways to disable a lint
+      // rule, we should instead search for a custom directive (like
+      // no-minify-errors) instead of ESLint. Will need to update our lint
+      // rule to recognize the same directive.
+      expect(
+        transform(`
+// eslint-disable-next-line react-internal/prod-error-codes
+Error('This is not a real error message.');
 `)
-    ).toMatchSnapshot();
-  });
+      ).toMatchSnapshot();
+    }
+  );
 
   it('should not touch other calls or new expressions', () => {
     expect(