Repository: facebook/react
Pull request: lint-build: Infer format from artifact filename

Uses the layout of the build artifact directory to infer the format of a given file, and which lint rules to apply.

This has the effect of decoupling the lint build job from the existing Rollup script, so that if we ever add additional post-processing, or if we replace Rollup, it will still work.

But the immediate motivation is to replace the separate "stable" and "experimental" lint-build jobs with a single combined job.

## Test plan

- Downloaded build artifacts
- Manually edited a few files to add a lint error (e.g. swapped a var for a const in `oss-stable/react/cjs/react.development.js`)
- Ran `yarn lint-build` to confirm the job fails in the expected way

Changed files:
filename: .circleci/config.yml
patch: @@ -280,20 +280,6 @@ jobs:
       - run: yarn lint-build
       - run: scripts/circleci/check_minified_errors.sh
 
-  RELEASE_CHANNEL_stable_yarn_lint_build:
-    docker: *docker
-    environment: *environment
-    steps:
-      - checkout
-      - attach_workspace: *attach_workspace
-      - run: yarn workspaces info | head -n -1 > workspace_info.txt
-      - *restore_node_modules
-      - run:
-          environment:
-            RELEASE_CHANNEL: stable
-          command: yarn lint-build
-      - run: scripts/circleci/check_minified_errors.sh
-
   yarn_test:
     docker: *docker
     environment: *environment
@@ -405,9 +391,6 @@ workflows:
       - RELEASE_CHANNEL_stable_yarn_build:
           requires:
             - setup
-      - RELEASE_CHANNEL_stable_yarn_lint_build:
-          requires:
-            - RELEASE_CHANNEL_stable_yarn_build
       - RELEASE_CHANNEL_stable_yarn_test_dom_fixtures:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
@@ -419,9 +402,6 @@ workflows:
       - yarn_build:
           requires:
             - setup
-      - yarn_lint_build:
-          requires:
-            - yarn_build
       - build_devtools_and_process_artifacts:
           requires:
             - yarn_build
@@ -522,6 +502,9 @@ workflows:
           requires:
             - get_base_build
             - yarn_build_combined
+      - yarn_lint_build:
+          requires:
+            - yarn_build_combined
   fuzz_tests:
     unless: << pipeline.parameters.prerelease_commit_sha >>
     triggers:

filename: scripts/circleci/check_minified_errors.sh
patch: @@ -2,7 +2,7 @@
 
 # Ensure errors are minified in production
 
-OUT=$(git --no-pager grep -n --untracked --no-exclude-standard 'FIXME (minify-errors-in-prod)' -- './build/*')
+OUT=$(git --no-pager grep -n --untracked --no-exclude-standard 'FIXME (minify-errors-in-prod)' -- './build2/*')
 
 if [ "$OUT" != "" ]; then
   echo "$OUT";

filename: scripts/rollup/validate/index.js
patch: @@ -1,59 +1,58 @@
 'use strict';
 
-const path = require('path');
+/* eslint-disable no-for-of-loops/no-for-of-loops */
 
+const path = require('path');
+const {promisify} = require('util');
+const glob = promisify(require('glob'));
 const {ESLint} = require('eslint');
 
-const {bundles, getFilename, bundleTypes} = require('../bundles');
-const Packaging = require('../packaging');
-
-const {
-  NODE_ES2015,
-  NODE_ESM,
-  UMD_DEV,
-  UMD_PROD,
-  UMD_PROFILING,
-  NODE_DEV,
-  NODE_PROD,
-  NODE_PROFILING,
-  FB_WWW_DEV,
-  FB_WWW_PROD,
-  FB_WWW_PROFILING,
-  RN_OSS_DEV,
-  RN_OSS_PROD,
-  RN_OSS_PROFILING,
-  RN_FB_DEV,
-  RN_FB_PROD,
-  RN_FB_PROFILING,
-} = bundleTypes;
+// Lint the final build artifacts. Helps catch bugs in our build pipeline.
 
-function getFormat(bundleType) {
-  switch (bundleType) {
-    case UMD_DEV:
-    case UMD_PROD:
-    case UMD_PROFILING:
-      return 'umd';
-    case NODE_ES2015:
+function getFormat(filepath) {
+  if (filepath.includes('facebook')) {
+    if (filepath.includes('shims')) {
+      // We don't currently lint these shims. We rely on the downstream Facebook
+      // repo to transform them.
+      // TODO: Should we lint them?
+      return null;
+    }
+    return 'fb';
+  }
+  if (filepath.includes('react-native')) {
+    if (filepath.includes('shims')) {
+      // We don't currently lint these shims. We rely on the downstream Facebook
+      // repo to transform them.
+      // TODO: Should we we lint them?
+      return null;
+    }
+    return 'rn';
+  }
+  if (filepath.includes('cjs')) {
+    if (
+      filepath.includes('react-server-dom-webpack-plugin') ||
+      filepath.includes('react-server-dom-webpack-node-register') ||
+      filepath.includes('react-suspense-test-utils')
+    ) {
       return 'cjs2015';
-    case NODE_ESM:
-      return 'esm';
-    case NODE_DEV:
-    case NODE_PROD:
-    case NODE_PROFILING:
-      return 'cjs';
-    case FB_WWW_DEV:
-    case FB_WWW_PROD:
-    case FB_WWW_PROFILING:
-      return 'fb';
-    case RN_OSS_DEV:
-    case RN_OSS_PROD:
-    case RN_OSS_PROFILING:
-    case RN_FB_DEV:
-    case RN_FB_PROD:
-    case RN_FB_PROFILING:
-      return 'rn';
+    }
+    return 'cjs';
+  }
+  if (filepath.includes('esm')) {
+    return 'esm';
+  }
+  if (filepath.includes('umd')) {
+    return 'umd';
   }
-  throw new Error('unknown bundleType');
+  if (
+    filepath.includes('oss-experimental') ||
+    filepath.includes('oss-stable')
+  ) {
+    // If a file in one of the open source channels doesn't match an earlier,
+    // more specific rule, then assume it's CommonJS.
+    return 'cjs';
+  }
+  throw new Error('Could not find matching lint format for file: ' + filepath);
 }
 
 function getESLintInstance(format) {
@@ -64,51 +63,42 @@ function getESLintInstance(format) {
   });
 }
 
-const esLints = {
-  cjs: getESLintInstance('cjs'),
-  cjs2015: getESLintInstance('cjs2015'),
-  esm: getESLintInstance('esm'),
-  rn: getESLintInstance('rn'),
-  fb: getESLintInstance('fb'),
-  umd: getESLintInstance('umd'),
-};
-
-// Performs sanity checks on bundles *built* by Rollup.
-// Helps catch Rollup regressions.
-async function lint(bundle, bundleType) {
-  const filename = getFilename(bundle, bundleType);
-  const format = getFormat(bundleType);
-  const eslint = esLints[format];
-
-  const packageName = Packaging.getPackageName(bundle.entry);
-  const mainOutputPath = Packaging.getBundleOutputPath(
-    bundleType,
-    filename,
-    packageName
-  );
-
-  const results = await eslint.lintFiles([mainOutputPath]);
+async function lint(eslint, filepaths) {
+  const results = await eslint.lintFiles(filepaths);
   if (
     results.some(result => result.errorCount > 0 || result.warningCount > 0)
   ) {
     process.exitCode = 1;
-    console.log(`Failed ${mainOutputPath}`);
+    console.log(`Lint failed`);
     const formatter = await eslint.loadFormatter('stylish');
     const resultText = formatter.format(results);
     console.log(resultText);
   }
 }
 
 async function lintEverything() {
-  console.log(`Linting known bundles...`);
-  let promises = [];
-  // eslint-disable-next-line no-for-of-loops/no-for-of-loops
-  for (const bundle of bundles) {
-    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
-    for (const bundleType of bundle.bundleTypes) {
-      promises.push(lint(bundle, bundleType));
+  console.log(`Linting build artifacts...`);
+
+  const allFilepaths = await glob('build2/**/*.js');
+
+  const pathsByFormat = new Map();
+  for (const filepath of allFilepaths) {
+    const format = getFormat(filepath);
+    if (format !== null) {
+      const paths = pathsByFormat.get(format);
+      if (paths === undefined) {
+        pathsByFormat.set(format, [filepath]);
+      } else {
+        paths.push(filepath);
+      }
     }
   }
+
+  const promises = [];
+  for (const [format, filepaths] of pathsByFormat) {
+    const eslint = getESLintInstance(format);
+    promises.push(lint(eslint, filepaths));
+  }
   await Promise.all(promises);
 }