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 a1543bf)

Author: vladar

packages/gatsby/src/utils/__tests__/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

packages/gatsby/src/utils/eslint-config.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<T = {}> = (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<string> }>
 
   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
-}

packages/gatsby/src/utils/webpack-utils.ts

@@ -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()
 }