Skip to content

Commit

Permalink
fix(gatsby): use separate eslint-loader for rules that are always req…
Browse files Browse the repository at this point in the history
…uired (#29317)

* fix(gatsby): use separate eslint-loader for rules that are always required

* Add the second loader only when custom eslint config is set
  • Loading branch information
vladar authored Feb 3, 2021
1 parent b2fec7c commit a1543bf
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 236 deletions.
130 changes: 0 additions & 130 deletions packages/gatsby/src/utils/__tests__/eslint-config.ts

This file was deleted.

46 changes: 5 additions & 41 deletions packages/gatsby/src/utils/eslint-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
73 changes: 21 additions & 52 deletions packages/gatsby/src/utils/webpack-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -69,6 +65,7 @@ interface ILoaderUtils {
exports: LoaderResolver

eslint(schema: GraphQLSchema): Loader
eslintRequired(): Loader
}

interface IModuleThatUseGatsby {
Expand Down Expand Up @@ -103,6 +100,7 @@ interface IRuleUtils {
postcss: ContextualRuleFactory<{ overrideBrowserOptions: Array<string> }>

eslint: (schema: GraphQLSchema) => RuleSetRule
eslintRequired: () => RuleSetRule
}

type PluginUtils = BuiltinPlugins & {
Expand Down Expand Up @@ -335,6 +333,13 @@ export const createWebpackUtils = (
}
},

eslintRequired: () => {
return {
options: eslintRequiredConfig,
loader: require.resolve(`eslint-loader`),
}
},

imports: (options = {}) => {
return {
options,
Expand Down Expand Up @@ -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$/,
Expand Down Expand Up @@ -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
}
26 changes: 13 additions & 13 deletions packages/gatsby/src/utils/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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()],
Expand Down Expand Up @@ -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()
}

0 comments on commit a1543bf

Please sign in to comment.