From b770f750054f85874bc6d483cc1dc01b4f6f5595 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 12 May 2021 10:14:45 -0700 Subject: [PATCH] lint-build: Infer format from artifact filename (#21489) 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. --- .circleci/config.yml | 23 +--- scripts/circleci/check_minified_errors.sh | 2 +- scripts/rollup/validate/index.js | 150 ++++++++++------------ 3 files changed, 74 insertions(+), 101 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9bfc78e5d32f7..d94cc684e371d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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: diff --git a/scripts/circleci/check_minified_errors.sh b/scripts/circleci/check_minified_errors.sh index df7673a6ead6a..df77ba0193ae1 100755 --- a/scripts/circleci/check_minified_errors.sh +++ b/scripts/circleci/check_minified_errors.sh @@ -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"; diff --git a/scripts/rollup/validate/index.js b/scripts/rollup/validate/index.js index 4d1c9210e231a..70921789d1998 100644 --- a/scripts/rollup/validate/index.js +++ b/scripts/rollup/validate/index.js @@ -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,35 +63,13 @@ 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); @@ -100,15 +77,28 @@ async function lint(bundle, bundleType) { } 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); }