feat(gatsby): add required eslint rules even if user has custom eslin…

…t config (#28911)

* feat(gatsby): add required eslint rules even if user has custom eslint config

* handle case when extends is single string, add some tests

* some extra existance checking

* more exact checks for existance of eslint-loader rule

* let's see if slash fixes win32 + env var for rules

* another try

* don't use slash after all

Co-authored-by: LekoArts <lekoarts@gmail.com>

packages/gatsby/src/utils/__tests__/eslint-config.ts

@@ -0,0 +1,130 @@
+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 [
+            "<PROJECT_ROOT>/packages/gatsby/src/utils/eslint/required.js",
+          ],
+        }
+      `)
+
+      expect(conf.rulePaths).toMatchInlineSnapshot(`
+        Array [
+          "<PROJECT_ROOT>/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",
+          "<PROJECT_ROOT>/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 [
+          "<PROJECT_ROOT>/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",
+            "<PROJECT_ROOT>/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",
+            "<PROJECT_ROOT>/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 [
+            "<PROJECT_ROOT>/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": "<PROJECT_ROOT>/packages/gatsby/src/utils/eslint/required.js",
+        }
+      `)
+    })
+  })
+})

packages/gatsby/src/utils/eslint-config.ts

@@ -1,5 +1,61 @@
 import { printSchema, GraphQLSchema } from "graphql"
 import { CLIEngine } from "eslint"
+import path from "path"
+
+const eslintRulePaths = path.resolve(`${__dirname}/eslint-rules`)
+const eslintRequirePreset = require.resolve(`./eslint/required`)
+
+export const eslintRequiredConfig: CLIEngine.Options = {
+  rulePaths: [eslintRulePaths],
+  baseConfig: {
+    globals: {
+      graphql: true,
+      __PATH_PREFIX__: true,
+      __BASE_PATH__: true, // this will rarely, if ever, be used by consumers
+    },
+    extends: [eslintRequirePreset],
+  },
+}
+
+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,
@@ -8,19 +64,19 @@ export const eslintConfig = (
   return {
     useEslintrc: false,
     resolvePluginsRelativeTo: __dirname,
-    rulePaths: [`${__dirname}/eslint-rules`],
+    rulePaths: [eslintRulePaths],
     baseConfig: {
       globals: {
         graphql: true,
         __PATH_PREFIX__: true,
         __BASE_PATH__: true, // this will rarely, if ever, be used by consumers
       },
-      extends: [require.resolve(`eslint-config-react-app`)],
+      extends: [
+        require.resolve(`eslint-config-react-app`),
+        eslintRequirePreset,
+      ],
       plugins: [`graphql`],
       rules: {
-        // Custom ESLint rules from Gatsby
-        "no-anonymous-exports-page-templates": `warn`,
-        "limited-exports-page-templates": `warn`,
         // New versions of react use a special jsx runtime that remove the requirement
         // for having react in scope for jsx. Once the jsx runtime is backported to all
         // versions of react we can make this always be `off`.

packages/gatsby/src/utils/eslint/required.js

@@ -0,0 +1,9 @@
+module.exports = {
+  rules: {
+    // Custom ESLint rules from Gatsby
+    "no-anonymous-exports-page-templates":
+      process.env.GATSBY_HOT_LOADER === `fast-refresh` ? `warn` : `off`,
+    "limited-exports-page-templates":
+      process.env.GATSBY_HOT_LOADER === `fast-refresh` ? `warn` : `off`,
+  },
+}

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

@@ -1,5 +1,5 @@
 import * as path from "path"
-import { Loader, RuleSetRule, Plugin } from "webpack"
+import { Loader, RuleSetRule, Plugin, Configuration } from "webpack"
 import { GraphQLSchema } from "graphql"
 import postcss from "postcss"
 import autoprefixer from "autoprefixer"
@@ -20,7 +20,11 @@ import {
 
 import { builtinPlugins } from "./webpack-plugins"
 import { IProgram, Stage } from "../commands/types"
-import { eslintConfig } from "./eslint-config"
+import {
+  eslintConfig,
+  mergeRequiredConfigIn,
+  eslintRequiredConfig,
+} from "./eslint-config"
 
 type LoaderResolver<T = {}> = (options?: T) => Loader
 
@@ -124,6 +128,8 @@ interface IWebpackUtils {
   plugins: PluginUtils
 }
 
+const vendorRegex = /(node_modules|bower_components)/
+
 /**
  * A factory method that produces an atoms namespace
  */
@@ -132,7 +138,6 @@ export const createWebpackUtils = (
   program: IProgram
 ): IWebpackUtils => {
   const assetRelativeRoot = `static/`
-  const vendorRegex = /(node_modules|bower_components)/
   const supportedBrowsers = getBrowsersList(program.directory)
 
   const PRODUCTION = !stage.includes(`develop`)
@@ -749,3 +754,50 @@ 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.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 } from "./webpack-utils"
+import { createWebpackUtils, ensureRequireEslintRules } from "./webpack-utils"
 import { hasLocalEslint } from "./local-eslint-config-finder"
 import { getAbsolutePathForVirtualModule } from "./gatsby-webpack-virtual-modules"
 
@@ -732,5 +732,15 @@ module.exports = async (
     parentSpan,
   })
 
-  return getConfig()
+  let finalConfig = getConfig()
+
+  if (
+    stage === `develop` &&
+    process.env.GATSBY_HOT_LOADER === `fast-refresh` &&
+    hasLocalEslint(program.directory)
+  ) {
+    finalConfig = ensureRequireEslintRules(finalConfig)
+  }
+
+  return finalConfig
 }