Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Feb 3, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
252c7c7
fix(gatsby): use separate eslint-loader for rules that are always req…
vladar Feb 3, 2021
063a87e
Add the second loader only when custom eslint config is set
vladar Feb 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
130 changes: 0 additions & 130 deletions packages/gatsby/src/utils/__tests__/eslint-config.ts
View file
Open in desktop

This file was deleted.

46 changes: 5 additions & 41 deletions packages/gatsby/src/utils/eslint-config.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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()])
}
Comment on lines +350 to +355
Copy link
Contributor Author

@vladar vladar Feb 3, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically the main piece. We add another minimal loader only when there is user-defined eslint config. When there is no custom config - we still use a single default Gatsby loader (with fast-refresh rules added to it)


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