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
2026-01-15 12:15:22 +00:00 · 2020-05-28 15:04:25 -07:00
parent 518ce9c25f
commit 60afa3c117
6 changed files with 161 additions and 120 deletions
--- a/.circleci/config.yml
+++ b/.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:
--- a/scripts/rollup/build.js
+++ b/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) {
--- a/scripts/rollup/bundles.js
+++ b/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,
 };
--- a/scripts/rollup/validate/eslintignore
+++ b/scripts/rollup/validate/eslintignore
@@ -1,11 +0,0 @@
-# Don't validate noop renderer bundle.
-# Unlike others, it currently uses ES6 syntax (generators).
-# We also don't use or publish it.
-**/react-noop-renderer/**
-JestReact-prod.js
-JestReact-dev.js
-**/jest-react/**
-# ESLint ignores `node_modules/*` in subdirectories
-# So we have to add this to lint build files.
-# https://github.com/eslint/eslint/pull/12888
-!/build/node_modules/*
--- a/scripts/rollup/validate/eslintrc.cjs.js
+++ b/scripts/rollup/validate/eslintrc.cjs.js
@@ -37,6 +37,9 @@ module.exports = {
    // Flight Webpack
    __webpack_chunk_load__: true,
    __webpack_require__: true,
+
+    // jest
+    expect: true,
  },
  parserOptions: {
    ecmaVersion: 5,
--- a/scripts/rollup/validate/index.js
+++ b/scripts/rollup/validate/index.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);
+});