Skip to content

Commit b770f75

Browse files
authored
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.
1 parent 2bf4805 commit b770f75

File tree

3 files changed

+74
-101
lines changed

3 files changed

+74
-101
lines changed

.circleci/config.yml

+3-20
Original file line numberDiff line numberDiff line change
@@ -280,20 +280,6 @@ jobs:
280280
- run: yarn lint-build
281281
- run: scripts/circleci/check_minified_errors.sh
282282

283-
RELEASE_CHANNEL_stable_yarn_lint_build:
284-
docker: *docker
285-
environment: *environment
286-
steps:
287-
- checkout
288-
- attach_workspace: *attach_workspace
289-
- run: yarn workspaces info | head -n -1 > workspace_info.txt
290-
- *restore_node_modules
291-
- run:
292-
environment:
293-
RELEASE_CHANNEL: stable
294-
command: yarn lint-build
295-
- run: scripts/circleci/check_minified_errors.sh
296-
297283
yarn_test:
298284
docker: *docker
299285
environment: *environment
@@ -405,9 +391,6 @@ workflows:
405391
- RELEASE_CHANNEL_stable_yarn_build:
406392
requires:
407393
- setup
408-
- RELEASE_CHANNEL_stable_yarn_lint_build:
409-
requires:
410-
- RELEASE_CHANNEL_stable_yarn_build
411394
- RELEASE_CHANNEL_stable_yarn_test_dom_fixtures:
412395
requires:
413396
- RELEASE_CHANNEL_stable_yarn_build
@@ -419,9 +402,6 @@ workflows:
419402
- yarn_build:
420403
requires:
421404
- setup
422-
- yarn_lint_build:
423-
requires:
424-
- yarn_build
425405
- build_devtools_and_process_artifacts:
426406
requires:
427407
- yarn_build
@@ -522,6 +502,9 @@ workflows:
522502
requires:
523503
- get_base_build
524504
- yarn_build_combined
505+
- yarn_lint_build:
506+
requires:
507+
- yarn_build_combined
525508
fuzz_tests:
526509
unless: << pipeline.parameters.prerelease_commit_sha >>
527510
triggers:

scripts/circleci/check_minified_errors.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
# Ensure errors are minified in production
44

5-
OUT=$(git --no-pager grep -n --untracked --no-exclude-standard 'FIXME (minify-errors-in-prod)' -- './build/*')
5+
OUT=$(git --no-pager grep -n --untracked --no-exclude-standard 'FIXME (minify-errors-in-prod)' -- './build2/*')
66

77
if [ "$OUT" != "" ]; then
88
echo "$OUT";

scripts/rollup/validate/index.js

+70-80
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,58 @@
11
'use strict';
22

3-
const path = require('path');
3+
/* eslint-disable no-for-of-loops/no-for-of-loops */
44

5+
const path = require('path');
6+
const {promisify} = require('util');
7+
const glob = promisify(require('glob'));
58
const {ESLint} = require('eslint');
69

7-
const {bundles, getFilename, bundleTypes} = require('../bundles');
8-
const Packaging = require('../packaging');
9-
10-
const {
11-
NODE_ES2015,
12-
NODE_ESM,
13-
UMD_DEV,
14-
UMD_PROD,
15-
UMD_PROFILING,
16-
NODE_DEV,
17-
NODE_PROD,
18-
NODE_PROFILING,
19-
FB_WWW_DEV,
20-
FB_WWW_PROD,
21-
FB_WWW_PROFILING,
22-
RN_OSS_DEV,
23-
RN_OSS_PROD,
24-
RN_OSS_PROFILING,
25-
RN_FB_DEV,
26-
RN_FB_PROD,
27-
RN_FB_PROFILING,
28-
} = bundleTypes;
10+
// Lint the final build artifacts. Helps catch bugs in our build pipeline.
2911

30-
function getFormat(bundleType) {
31-
switch (bundleType) {
32-
case UMD_DEV:
33-
case UMD_PROD:
34-
case UMD_PROFILING:
35-
return 'umd';
36-
case NODE_ES2015:
12+
function getFormat(filepath) {
13+
if (filepath.includes('facebook')) {
14+
if (filepath.includes('shims')) {
15+
// We don't currently lint these shims. We rely on the downstream Facebook
16+
// repo to transform them.
17+
// TODO: Should we lint them?
18+
return null;
19+
}
20+
return 'fb';
21+
}
22+
if (filepath.includes('react-native')) {
23+
if (filepath.includes('shims')) {
24+
// We don't currently lint these shims. We rely on the downstream Facebook
25+
// repo to transform them.
26+
// TODO: Should we we lint them?
27+
return null;
28+
}
29+
return 'rn';
30+
}
31+
if (filepath.includes('cjs')) {
32+
if (
33+
filepath.includes('react-server-dom-webpack-plugin') ||
34+
filepath.includes('react-server-dom-webpack-node-register') ||
35+
filepath.includes('react-suspense-test-utils')
36+
) {
3737
return 'cjs2015';
38-
case NODE_ESM:
39-
return 'esm';
40-
case NODE_DEV:
41-
case NODE_PROD:
42-
case NODE_PROFILING:
43-
return 'cjs';
44-
case FB_WWW_DEV:
45-
case FB_WWW_PROD:
46-
case FB_WWW_PROFILING:
47-
return 'fb';
48-
case RN_OSS_DEV:
49-
case RN_OSS_PROD:
50-
case RN_OSS_PROFILING:
51-
case RN_FB_DEV:
52-
case RN_FB_PROD:
53-
case RN_FB_PROFILING:
54-
return 'rn';
38+
}
39+
return 'cjs';
40+
}
41+
if (filepath.includes('esm')) {
42+
return 'esm';
43+
}
44+
if (filepath.includes('umd')) {
45+
return 'umd';
5546
}
56-
throw new Error('unknown bundleType');
47+
if (
48+
filepath.includes('oss-experimental') ||
49+
filepath.includes('oss-stable')
50+
) {
51+
// If a file in one of the open source channels doesn't match an earlier,
52+
// more specific rule, then assume it's CommonJS.
53+
return 'cjs';
54+
}
55+
throw new Error('Could not find matching lint format for file: ' + filepath);
5756
}
5857

5958
function getESLintInstance(format) {
@@ -64,51 +63,42 @@ function getESLintInstance(format) {
6463
});
6564
}
6665

67-
const esLints = {
68-
cjs: getESLintInstance('cjs'),
69-
cjs2015: getESLintInstance('cjs2015'),
70-
esm: getESLintInstance('esm'),
71-
rn: getESLintInstance('rn'),
72-
fb: getESLintInstance('fb'),
73-
umd: getESLintInstance('umd'),
74-
};
75-
76-
// Performs sanity checks on bundles *built* by Rollup.
77-
// Helps catch Rollup regressions.
78-
async function lint(bundle, bundleType) {
79-
const filename = getFilename(bundle, bundleType);
80-
const format = getFormat(bundleType);
81-
const eslint = esLints[format];
82-
83-
const packageName = Packaging.getPackageName(bundle.entry);
84-
const mainOutputPath = Packaging.getBundleOutputPath(
85-
bundleType,
86-
filename,
87-
packageName
88-
);
89-
90-
const results = await eslint.lintFiles([mainOutputPath]);
66+
async function lint(eslint, filepaths) {
67+
const results = await eslint.lintFiles(filepaths);
9168
if (
9269
results.some(result => result.errorCount > 0 || result.warningCount > 0)
9370
) {
9471
process.exitCode = 1;
95-
console.log(`Failed ${mainOutputPath}`);
72+
console.log(`Lint failed`);
9673
const formatter = await eslint.loadFormatter('stylish');
9774
const resultText = formatter.format(results);
9875
console.log(resultText);
9976
}
10077
}
10178

10279
async function lintEverything() {
103-
console.log(`Linting known bundles...`);
104-
let promises = [];
105-
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
106-
for (const bundle of bundles) {
107-
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
108-
for (const bundleType of bundle.bundleTypes) {
109-
promises.push(lint(bundle, bundleType));
80+
console.log(`Linting build artifacts...`);
81+
82+
const allFilepaths = await glob('build2/**/*.js');
83+
84+
const pathsByFormat = new Map();
85+
for (const filepath of allFilepaths) {
86+
const format = getFormat(filepath);
87+
if (format !== null) {
88+
const paths = pathsByFormat.get(format);
89+
if (paths === undefined) {
90+
pathsByFormat.set(format, [filepath]);
91+
} else {
92+
paths.push(filepath);
93+
}
11094
}
11195
}
96+
97+
const promises = [];
98+
for (const [format, filepaths] of pathsByFormat) {
99+
const eslint = getESLintInstance(format);
100+
promises.push(lint(eslint, filepaths));
101+
}
112102
await Promise.all(promises);
113103
}
114104

0 commit comments

Comments
 (0)