From 60afa3c11761e62d98c90b036890372c61bcc4c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 28 May 2020 15:04:25 -0700 Subject: [PATCH] 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 --- .circleci/config.yml | 16 ++- scripts/rollup/build.js | 49 ++------ scripts/rollup/bundles.js | 42 ++++++- scripts/rollup/validate/eslintignore | 11 -- scripts/rollup/validate/eslintrc.cjs.js | 3 + scripts/rollup/validate/index.js | 160 ++++++++++++++---------- 6 files changed, 161 insertions(+), 120 deletions(-) delete mode 100644 scripts/rollup/validate/eslintignore diff --git a/.circleci/config.yml b/.circleci/config.yml index 90aa299677..5083e34651 100644 --- 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: diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index 92375e7bfe..414f4ee6c3 100644 --- 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) { diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index f9c32cd51c..12b01f9911 100644 --- 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, }; diff --git a/scripts/rollup/validate/eslintignore b/scripts/rollup/validate/eslintignore deleted file mode 100644 index 34b9063007..0000000000 --- a/scripts/rollup/validate/eslintignore +++ /dev/null @@ -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/* diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 008cbf1d78..0e182bfefc 100644 --- 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, diff --git a/scripts/rollup/validate/index.js b/scripts/rollup/validate/index.js index a487721624..b6d06c17a7 100644 --- 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); +});