Lint bundles using the bundle config instead of scanning for files (#19025)

* Lint bundles using the bundle config instead of scanning for files

This ensures that we look for all the files that we expect to see there.
If something doesn't get built we wouldn't detect it.

However, this doesn't find files that aren't part of our builds such as
indirection files in the root. This will need to change with ESM anyway
since indirection files doesn't work. Everything should be built anyway.

This ensures that we can use the bundles.js config to determine special
cases instead of relying on file system conventions.

* Run lint with flag

Author: sebmarkbage

.circleci/config.yml

@@ -364,6 +364,20 @@ 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
+      - *restore_yarn_cache
+      - *run_yarn
+      - run:
+          environment:
+            RELEASE_CHANNEL: stable
+          command: yarn lint-build
+      - run: scripts/circleci/check_minified_errors.sh
+
   RELEASE_CHANNEL_stable_yarn_test_build:
     docker: *docker
     environment: *environment
@@ -500,7 +514,7 @@ workflows:
       - sizebot_stable:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
-      - yarn_lint_build:
+      - RELEASE_CHANNEL_stable_yarn_lint_build:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
       - RELEASE_CHANNEL_stable_yarn_test_build:

scripts/rollup/build.js

@@ -62,6 +62,8 @@ const {
   RN_FB_PROFILING,
 } = Bundles.bundleTypes;
 
+const {getFilename} = Bundles;
+
 function parseRequestedNames(names, toCase) {
   let result = [];
   for (let i = 0; i < names.length; i++) {
@@ -258,37 +260,6 @@ function getFormat(bundleType) {
   }
 }
 
-function getFilename(name, globalName, bundleType) {
-  // we do this to replace / to -, for react-dom/server
-  name = name.replace('/index.', '.').replace('/', '-');
-  switch (bundleType) {
-    case UMD_DEV:
-      return `${name}.development.js`;
-    case UMD_PROD:
-      return `${name}.production.min.js`;
-    case UMD_PROFILING:
-      return `${name}.profiling.min.js`;
-    case NODE_DEV:
-      return `${name}.development.js`;
-    case NODE_PROD:
-      return `${name}.production.min.js`;
-    case NODE_PROFILING:
-      return `${name}.profiling.min.js`;
-    case FB_WWW_DEV:
-    case RN_OSS_DEV:
-    case RN_FB_DEV:
-      return `${globalName}-dev.js`;
-    case FB_WWW_PROD:
-    case RN_OSS_PROD:
-    case RN_FB_PROD:
-      return `${globalName}-prod.js`;
-    case FB_WWW_PROFILING:
-    case RN_FB_PROFILING:
-    case RN_OSS_PROFILING:
-      return `${globalName}-profiling.js`;
-  }
-}
-
 function isProductionBundleType(bundleType) {
   switch (bundleType) {
     case UMD_DEV:
@@ -558,7 +529,7 @@ async function createBundle(bundle, bundleType) {
     return;
   }
 
-  const filename = getFilename(bundle.entry, bundle.global, bundleType);
+  const filename = getFilename(bundle, bundleType);
   const logKey =
     chalk.white.bold(filename) + chalk.dim(` (${bundleType.toLowerCase()})`);
   const format = getFormat(bundleType);
@@ -765,17 +736,11 @@ async function buildEverything() {
       [bundle, FB_WWW_PROFILING],
       [bundle, RN_OSS_DEV],
       [bundle, RN_OSS_PROD],
-      [bundle, RN_OSS_PROFILING]
+      [bundle, RN_OSS_PROFILING],
+      [bundle, RN_FB_DEV],
+      [bundle, RN_FB_PROD],
+      [bundle, RN_FB_PROFILING]
     );
-
-    if (__EXPERIMENTAL__) {
-      // FB-specific RN builds are experimental-only.
-      bundles.push(
-        [bundle, RN_FB_DEV],
-        [bundle, RN_FB_PROD],
-        [bundle, RN_FB_PROFILING]
-      );
-    }
   }
 
   if (!shouldExtractErrors && process.env.CIRCLE_NODE_TOTAL) {

scripts/rollup/bundles.js

@@ -356,7 +356,9 @@ const bundles = [
 
   /******* React Native *******/
   {
-    bundleTypes: [RN_FB_DEV, RN_FB_PROD, RN_FB_PROFILING],
+    bundleTypes: __EXPERIMENTAL__
+      ? [RN_FB_DEV, RN_FB_PROD, RN_FB_PROFILING]
+      : [],
     moduleType: RENDERER,
     entry: 'react-native-renderer',
     global: 'ReactNativeRenderer',
@@ -384,7 +386,9 @@ const bundles = [
 
   /******* React Native Fabric *******/
   {
-    bundleTypes: [RN_FB_DEV, RN_FB_PROD, RN_FB_PROFILING],
+    bundleTypes: __EXPERIMENTAL__
+      ? [RN_FB_DEV, RN_FB_PROD, RN_FB_PROFILING]
+      : [],
     moduleType: RENDERER,
     entry: 'react-native-renderer/fabric',
     global: 'ReactFabric',
@@ -793,9 +797,43 @@ deepFreeze(bundles);
 deepFreeze(bundleTypes);
 deepFreeze(moduleTypes);
 
+function getFilename(bundle, bundleType) {
+  let name = bundle.entry;
+  const globalName = bundle.global;
+  // we do this to replace / to -, for react-dom/server
+  name = name.replace('/index.', '.').replace('/', '-');
+  switch (bundleType) {
+    case UMD_DEV:
+      return `${name}.development.js`;
+    case UMD_PROD:
+      return `${name}.production.min.js`;
+    case UMD_PROFILING:
+      return `${name}.profiling.min.js`;
+    case NODE_DEV:
+      return `${name}.development.js`;
+    case NODE_PROD:
+      return `${name}.production.min.js`;
+    case NODE_PROFILING:
+      return `${name}.profiling.min.js`;
+    case FB_WWW_DEV:
+    case RN_OSS_DEV:
+    case RN_FB_DEV:
+      return `${globalName}-dev.js`;
+    case FB_WWW_PROD:
+    case RN_OSS_PROD:
+    case RN_FB_PROD:
+      return `${globalName}-prod.js`;
+    case FB_WWW_PROFILING:
+    case RN_FB_PROFILING:
+    case RN_OSS_PROFILING:
+      return `${globalName}-profiling.js`;
+  }
+}
+
 module.exports = {
   fbBundleExternalsMap,
   bundleTypes,
   moduleTypes,
   bundles,
+  getFilename,
 };

scripts/rollup/validate/eslintignore

@@ -37,6 +37,9 @@ module.exports = {
     // Flight Webpack
     __webpack_chunk_load__: true,
     __webpack_require__: true,
+
+    // jest
+    expect: true,
   },
   parserOptions: {
     ecmaVersion: 5,

scripts/rollup/validate/eslintrc.cjs.js

@@ -1,78 +1,110 @@
 'use strict';
 
-const chalk = require('chalk');
 const path = require('path');
-const spawnSync = require('child_process').spawnSync;
-const glob = require('glob');
 
-const extension = process.platform === 'win32' ? '.cmd' : '';
+const {ESLint} = require('eslint');
+
+const {bundles, getFilename, bundleTypes} = require('../bundles');
+const Packaging = require('../packaging');
+
+const {
+  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;
+
+function getFormat(bundleType) {
+  switch (bundleType) {
+    case UMD_DEV:
+    case UMD_PROD:
+    case UMD_PROFILING:
+      return 'umd';
+    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';
+  }
+  throw new Error('unknown bundleType');
+}
+
+function getESLintInstance(format) {
+  return new ESLint({
+    useEslintrc: false,
+    overrideConfigFile: path.join(__dirname, `eslintrc.${format}.js`),
+    ignore: false,
+  });
+}
+
+const esLints = {
+  cjs: getESLintInstance('cjs'),
+  rn: getESLintInstance('rn'),
+  fb: getESLintInstance('fb'),
+  umd: getESLintInstance('umd'),
+};
 
 // Performs sanity checks on bundles *built* by Rollup.
 // Helps catch Rollup regressions.
-function lint({format, filePatterns}) {
-  console.log(`Linting ${format} bundles...`);
-  const result = spawnSync(
-    path.join('node_modules', '.bin', 'eslint' + extension),
-    [
-      ...filePatterns,
-      '--config',
-      path.join(__dirname, `eslintrc.${format}.js`),
-      // Disregard our ESLint rules that apply to the source.
-      '--no-eslintrc',
-      // Use a different ignore file.
-      '--ignore-path',
-      path.join(__dirname, 'eslintignore'),
-    ],
-    {
-      // Allow colors to pass through
-      stdio: 'inherit',
-    }
+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
   );
-  if (result.status !== 0) {
-    console.error(chalk.red(`Linting of ${format} bundles has failed.`));
-    process.exit(result.status);
-  } else {
-    console.log(chalk.green(`Linted ${format} bundles successfully!`));
-    console.log();
+
+  const results = await eslint.lintFiles([mainOutputPath]);
+  if (
+    results.some(result => result.errorCount > 0 || result.warningCount > 0)
+  ) {
+    process.exitCode = 1;
+    console.log(`Failed ${mainOutputPath}`);
+    const formatter = await eslint.loadFormatter('stylish');
+    const resultText = formatter.format(results);
+    console.log(resultText);
   }
 }
 
-function checkFilesExist(bundle) {
-  const {format, filePatterns} = bundle;
-  filePatterns.forEach(pattern => {
-    console.log(`Checking if files exist in ${pattern}...`);
-    const files = glob.sync(pattern);
-    if (files.length === 0) {
-      console.error(chalk.red(`Found no ${format} bundles in ${pattern}`));
-      process.exit(1);
-    } else {
-      console.log(chalk.green(`Found ${files.length} bundles.`));
-      console.log();
+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));
     }
-  });
-  return bundle;
+  }
+  await Promise.all(promises);
 }
 
-const bundles = [
-  {
-    format: 'rn',
-    filePatterns: [`./build/react-native/implementations/*.js`],
-  },
-  {
-    format: 'umd',
-    filePatterns: [`./build/node_modules/*/umd/*.js`],
-  },
-  {
-    format: 'cjs',
-    filePatterns: [
-      `./build/node_modules/*/*.js`,
-      `./build/node_modules/*/cjs/*.js`,
-    ],
-  },
-  {
-    format: 'fb',
-    filePatterns: [`./build/facebook-www/*.js`],
-  },
-];
-
-bundles.map(checkFilesExist).map(lint);
+lintEverything().catch(error => {
+  process.exitCode = 1;
+  console.error(error);
+});