Skip to content

Commit

Permalink
lint-build: Infer format from artifact filename (#21489)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed May 12, 2021
1 parent 2bf4805 commit b770f75
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 101 deletions.
23 changes: 3 additions & 20 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

This comment has been minimized.

Copy link
@771892

771892 Jun 4, 2021

attach_workspace

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

This comment has been minimized.

Copy link
@771892

771892 Jun 4, 2021

checkout in .sh file


yarn_test:
docker: *docker
environment: *environment
Expand Down Expand Up @@ -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
Expand All @@ -419,9 +402,6 @@ workflows:
- yarn_build:
requires:
- setup
- yarn_lint_build:
requires:
- yarn_build
- build_devtools_and_process_artifacts:
requires:
- yarn_build
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion scripts/circleci/check_minified_errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
150 changes: 70 additions & 80 deletions scripts/rollup/validate/index.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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);
}

Expand Down

0 comments on commit b770f75

Please sign in to comment.