From 2fc0a3c4b7e9c39479ee86db11c8cb054b7ffd5f Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Thu, 4 Feb 2021 03:29:05 +0700 Subject: [PATCH] fix(gatsby): use separate eslint-loader for rules that are always required (#29317) * fix(gatsby): use separate eslint-loader for rules that are always required * Add the second loader only when custom eslint config is set (cherry picked from commit a1543bf535871a921fdd2e9893abce064cbe5565) --- .../src/utils/__tests__/eslint-config.ts | 130 ------------------ packages/gatsby/src/utils/eslint-config.ts | 46 +------ packages/gatsby/src/utils/webpack-utils.ts | 73 +++------- packages/gatsby/src/utils/webpack.config.js | 26 ++-- 4 files changed, 39 insertions(+), 236 deletions(-) delete mode 100644 packages/gatsby/src/utils/__tests__/eslint-config.ts diff --git a/packages/gatsby/src/utils/__tests__/eslint-config.ts b/packages/gatsby/src/utils/__tests__/eslint-config.ts deleted file mode 100644 index e9fdf61f41822..0000000000000 --- a/packages/gatsby/src/utils/__tests__/eslint-config.ts +++ /dev/null @@ -1,130 +0,0 @@ -import { mergeRequiredConfigIn } from "../eslint-config" -import { CLIEngine } from "eslint" -import * as path from "path" - -describe(`eslint-config`, () => { - describe(`mergeRequiredConfigIn`, () => { - it(`adds rulePaths and extends if those don't exist`, () => { - const conf: CLIEngine.Options = {} - - mergeRequiredConfigIn(conf) - - expect(conf?.baseConfig).toMatchInlineSnapshot(` - Object { - "extends": Array [ - "/packages/gatsby/src/utils/eslint/required.js", - ], - } - `) - - expect(conf.rulePaths).toMatchInlineSnapshot(` - Array [ - "/packages/gatsby/src/utils/eslint-rules", - ] - `) - }) - - it(`adds rulePath if rulePaths exist but don't contain required rules`, () => { - const conf: CLIEngine.Options = { - rulePaths: [`test`], - } - - mergeRequiredConfigIn(conf) - - expect(conf.rulePaths).toMatchInlineSnapshot(` - Array [ - "test", - "/packages/gatsby/src/utils/eslint-rules", - ] - `) - }) - - it(`doesn't add rulePath multiple times`, () => { - const conf: CLIEngine.Options = { - rulePaths: [path.resolve(__dirname, `../eslint-rules`), `test`], - } - - mergeRequiredConfigIn(conf) - - expect(conf.rulePaths).toMatchInlineSnapshot(` - Array [ - "/packages/gatsby/src/utils/eslint-rules", - "test", - ] - `) - }) - - it(`adds extend if extends exist (array) but don't contain required preset`, () => { - const conf: CLIEngine.Options = { - baseConfig: { - extends: [`ext1`], - }, - } - - mergeRequiredConfigIn(conf) - - expect(conf.baseConfig).toMatchInlineSnapshot(` - Object { - "extends": Array [ - "ext1", - "/packages/gatsby/src/utils/eslint/required.js", - ], - } - `) - }) - - it(`adds extend if extends exist (string) but don't contain required preset`, () => { - const conf: CLIEngine.Options = { - baseConfig: { - extends: `ext1`, - }, - } - - mergeRequiredConfigIn(conf) - - expect(conf.baseConfig).toMatchInlineSnapshot(` - Object { - "extends": Array [ - "ext1", - "/packages/gatsby/src/utils/eslint/required.js", - ], - } - `) - }) - - it(`doesn't add extend multiple times (extends is array)`, () => { - const conf: CLIEngine.Options = { - baseConfig: { - extends: [require.resolve(`../eslint/required`), `ext1`], - }, - } - - mergeRequiredConfigIn(conf) - - expect(conf.baseConfig).toMatchInlineSnapshot(` - Object { - "extends": Array [ - "/packages/gatsby/src/utils/eslint/required.js", - "ext1", - ], - } - `) - }) - - it(`doesn't add extend multiple times (extends is string)`, () => { - const conf: CLIEngine.Options = { - baseConfig: { - extends: require.resolve(`../eslint/required`), - }, - } - - mergeRequiredConfigIn(conf) - - expect(conf.baseConfig).toMatchInlineSnapshot(` - Object { - "extends": "/packages/gatsby/src/utils/eslint/required.js", - } - `) - }) - }) -}) diff --git a/packages/gatsby/src/utils/eslint-config.ts b/packages/gatsby/src/utils/eslint-config.ts index b9a35a9f3b5dd..daf942dcb4ae7 100644 --- a/packages/gatsby/src/utils/eslint-config.ts +++ b/packages/gatsby/src/utils/eslint-config.ts @@ -8,9 +8,13 @@ const eslintRequirePreset = require.resolve(`./eslint/required`) export const eslintRequiredConfig: CLIEngine.Options = { rulePaths: [eslintRulePaths], useEslintrc: false, + allowInlineConfig: false, + // @ts-ignore + emitWarning: true, baseConfig: { + parser: require.resolve(`babel-eslint`), parserOptions: { - ecmaVersion: 2018, + ecmaVersion: 2020, sourceType: `module`, ecmaFeatures: { jsx: true, @@ -25,46 +29,6 @@ export const eslintRequiredConfig: CLIEngine.Options = { }, } -export function mergeRequiredConfigIn( - existingOptions: CLIEngine.Options -): void { - // make sure rulePaths include our custom rules - if (existingOptions.rulePaths) { - if ( - Array.isArray(existingOptions.rulePaths) && - !existingOptions.rulePaths.includes(eslintRulePaths) - ) { - existingOptions.rulePaths.push(eslintRulePaths) - } - } else { - existingOptions.rulePaths = [eslintRulePaths] - } - - // make sure we extend required preset - if (!existingOptions.baseConfig) { - existingOptions.baseConfig = {} - } - - if (existingOptions.baseConfig.extends) { - if ( - Array.isArray(existingOptions.baseConfig.extends) && - !existingOptions.baseConfig.extends.includes(eslintRequirePreset) - ) { - existingOptions.baseConfig.extends.push(eslintRequirePreset) - } else if ( - typeof existingOptions.baseConfig.extends === `string` && - existingOptions.baseConfig.extends !== eslintRequirePreset - ) { - existingOptions.baseConfig.extends = [ - existingOptions.baseConfig.extends, - eslintRequirePreset, - ] - } - } else { - existingOptions.baseConfig.extends = [eslintRequirePreset] - } -} - export const eslintConfig = ( schema: GraphQLSchema, usingJsxRuntime: boolean diff --git a/packages/gatsby/src/utils/webpack-utils.ts b/packages/gatsby/src/utils/webpack-utils.ts index 42255099e15b0..dcb6fee9253c0 100644 --- a/packages/gatsby/src/utils/webpack-utils.ts +++ b/packages/gatsby/src/utils/webpack-utils.ts @@ -20,11 +20,7 @@ import { import { builtinPlugins } from "./webpack-plugins" import { IProgram, Stage } from "../commands/types" -import { - eslintConfig, - mergeRequiredConfigIn, - eslintRequiredConfig, -} from "./eslint-config" +import { eslintConfig, eslintRequiredConfig } from "./eslint-config" type LoaderResolver = (options?: T) => Loader @@ -69,6 +65,7 @@ interface ILoaderUtils { exports: LoaderResolver eslint(schema: GraphQLSchema): Loader + eslintRequired(): Loader } interface IModuleThatUseGatsby { @@ -103,6 +100,7 @@ interface IRuleUtils { postcss: ContextualRuleFactory<{ overrideBrowserOptions: Array }> eslint: (schema: GraphQLSchema) => RuleSetRule + eslintRequired: () => RuleSetRule } type PluginUtils = BuiltinPlugins & { @@ -335,6 +333,13 @@ export const createWebpackUtils = ( } }, + eslintRequired: () => { + return { + options: eslintRequiredConfig, + loader: require.resolve(`eslint-loader`), + } + }, + imports: (options = {}) => { return { options, @@ -496,6 +501,17 @@ export const createWebpackUtils = ( } } + rules.eslintRequired = (): RuleSetRule => { + return { + enforce: `pre`, + test: /\.jsx?$/, + exclude: (modulePath: string): boolean => + modulePath.includes(VIRTUAL_MODULES_BASE_PATH) || + vendorRegex.test(modulePath), + use: [loaders.eslintRequired()], + } + } + rules.yaml = (): RuleSetRule => { return { test: /\.ya?ml$/, @@ -754,50 +770,3 @@ export function reactHasJsxRuntime(): boolean { return false } - -export function ensureRequireEslintRules(config: Configuration): Configuration { - if (!config.module) { - config.module = { - rules: [], - } - } - // for fast refresh we want to ensure that that there is eslint rule running - // because user might have added their own `eslint-loader` let's check if there is one - // and adjust it to add the rule or append new loader with required rule - const rule = config.module.rules.find(rule => { - if (typeof rule.loader === `string`) { - return ( - rule.loader === `eslint-loader` || - rule.loader.endsWith(`eslint-loader/index.js`) || - rule.loader.endsWith(`eslint-loader/dist/cjs.js`) - ) - } - - return false - }) - - if (rule) { - if (typeof rule.options !== `string`) { - if (!rule.options) { - rule.options = {} - } - mergeRequiredConfigIn(rule.options) - } - } else { - config.module.rules.push({ - enforce: `pre`, - test: /\.jsx?$/, - exclude: (modulePath: string): boolean => - modulePath.includes(VIRTUAL_MODULES_BASE_PATH) || - vendorRegex.test(modulePath), - use: [ - { - loader: require.resolve(`eslint-loader`), - options: eslintRequiredConfig, - }, - ], - }) - } - - return config -} diff --git a/packages/gatsby/src/utils/webpack.config.js b/packages/gatsby/src/utils/webpack.config.js index 9db7273c17770..ec0b1fe506d79 100644 --- a/packages/gatsby/src/utils/webpack.config.js +++ b/packages/gatsby/src/utils/webpack.config.js @@ -15,7 +15,7 @@ const report = require(`gatsby-cli/lib/reporter`) import { withBasePath, withTrailingSlash } from "./path" import { getGatsbyDependents } from "./gatsby-dependents" const apiRunnerNode = require(`./api-runner-node`) -import { createWebpackUtils, ensureRequireEslintRules } from "./webpack-utils" +import { createWebpackUtils } from "./webpack-utils" import { hasLocalEslint } from "./local-eslint-config-finder" import { getAbsolutePathForVirtualModule } from "./gatsby-webpack-virtual-modules" @@ -339,11 +339,21 @@ module.exports = async ( // get schema to pass to eslint config and program for directory const { schema, program } = store.getState() + const isCustomEslint = hasLocalEslint(program.directory) + // if no local eslint config, then add gatsby config - if (!hasLocalEslint(program.directory)) { + if (!isCustomEslint) { configRules = configRules.concat([rules.eslint(schema)]) } + // Enforce fast-refresh rules even with local eslint config + if ( + isCustomEslint && + process.env.GATSBY_HOT_LOADER === `fast-refresh` + ) { + configRules = configRules.concat([rules.eslintRequired()]) + } + configRules = configRules.concat([ { oneOf: [rules.cssModules(), rules.css()], @@ -732,15 +742,5 @@ module.exports = async ( parentSpan, }) - let finalConfig = getConfig() - - if ( - stage === `develop` && - process.env.GATSBY_HOT_LOADER === `fast-refresh` && - hasLocalEslint(program.directory) - ) { - finalConfig = ensureRequireEslintRules(finalConfig) - } - - return finalConfig + return getConfig() }