From 9a55d1231331ebd8e1fab5496a85ba0a14dacdca Mon Sep 17 00:00:00 2001 From: Lennart Date: Fri, 15 Jan 2021 16:01:36 +0100 Subject: [PATCH] feat(gatsby): Add eslint rules to warn against bad patterns in pageTemplates (for Fast Refresh) (#28689) Co-authored-by: LekoArts Co-authored-by: Peter van der Zee <209817+pvdz@users.noreply.github.com> Co-authored-by: Michal Piechowiak Co-authored-by: gatsbybot --- .../limited-exports-page-templates.js | 14 ++ .../no-anonymous-exports-page-templates.js | 22 ++ .../limited-exports-page-templates.js | 27 +++ ...onymous-exports-page-templates-function.js | 11 + .../no-anonymous-exports-page-templates.js | 9 + .../src/utils/__tests__/eslint-config.ts | 130 +++++++++++ packages/gatsby/src/utils/eslint-config.ts | 66 +++++- .../gatsby/src/utils/eslint-rules-helpers.ts | 22 ++ .../limited-exports-page-templates.ts | 91 ++++++++ .../no-anonymous-exports-page-templates.ts | 53 +++++ .../limited-exports-page-templates.ts | 204 ++++++++++++++++++ .../no-anonymous-exports-page-templates.ts | 79 +++++++ packages/gatsby/src/utils/eslint/required.js | 9 + packages/gatsby/src/utils/webpack-utils.ts | 58 ++++- packages/gatsby/src/utils/webpack.config.js | 14 +- 15 files changed, 801 insertions(+), 8 deletions(-) create mode 100644 e2e-tests/development-runtime/cypress/integration/eslint-rules/limited-exports-page-templates.js create mode 100644 e2e-tests/development-runtime/cypress/integration/eslint-rules/no-anonymous-exports-page-templates.js create mode 100644 e2e-tests/development-runtime/src/pages/eslint-rules/limited-exports-page-templates.js create mode 100644 e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates-function.js create mode 100644 e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates.js create mode 100644 packages/gatsby/src/utils/__tests__/eslint-config.ts create mode 100644 packages/gatsby/src/utils/eslint-rules-helpers.ts create mode 100644 packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts create mode 100644 packages/gatsby/src/utils/eslint-rules/__tests__/no-anonymous-exports-page-templates.ts create mode 100644 packages/gatsby/src/utils/eslint-rules/limited-exports-page-templates.ts create mode 100644 packages/gatsby/src/utils/eslint-rules/no-anonymous-exports-page-templates.ts create mode 100644 packages/gatsby/src/utils/eslint/required.js diff --git a/e2e-tests/development-runtime/cypress/integration/eslint-rules/limited-exports-page-templates.js b/e2e-tests/development-runtime/cypress/integration/eslint-rules/limited-exports-page-templates.js new file mode 100644 index 0000000000000..3464d8de60a7a --- /dev/null +++ b/e2e-tests/development-runtime/cypress/integration/eslint-rules/limited-exports-page-templates.js @@ -0,0 +1,14 @@ +if (Cypress.env("HOT_LOADER") === `fast-refresh`) { + describe(`limited-exports-page-templates`, () => { + it(`should log warning to console for invalid export`, () => { + cy.visit(`/eslint-rules/limited-exports-page-templates`, { + onBeforeLoad(win) { + cy.stub(win.console, 'log').as(`consoleLog`) + } + }).waitForRouteChange() + + cy.get(`@consoleLog`).should(`be.calledWithMatch`, /15:1 warning In page templates only a default export of a valid React component and the named export of a page query is allowed./i) + cy.get(`@consoleLog`).should(`not.be.calledWithMatch`, /17:1 warning In page templates only a default export of a valid React component and the named export of a page query is allowed./i) + }) + }) +} diff --git a/e2e-tests/development-runtime/cypress/integration/eslint-rules/no-anonymous-exports-page-templates.js b/e2e-tests/development-runtime/cypress/integration/eslint-rules/no-anonymous-exports-page-templates.js new file mode 100644 index 0000000000000..53aacb74fea8b --- /dev/null +++ b/e2e-tests/development-runtime/cypress/integration/eslint-rules/no-anonymous-exports-page-templates.js @@ -0,0 +1,22 @@ +if (Cypress.env("HOT_LOADER") === `fast-refresh`) { + describe(`no-anonymous-exports-page-templates`, () => { + it(`should log warning to console for arrow functions`, () => { + cy.visit(`/eslint-rules/no-anonymous-exports-page-templates`, { + onBeforeLoad(win) { + cy.stub(win.console, 'log').as(`consoleLog`) + } + }).waitForRouteChange() + + cy.get(`@consoleLog`).should(`be.calledWithMatch`, /Anonymous arrow functions cause Fast Refresh to not preserve local component state./i) + }) + it(`should log warning to console for function declarations`, () => { + cy.visit(`/eslint-rules/no-anonymous-exports-page-templates-function`, { + onBeforeLoad(win) { + cy.stub(win.console, 'log').as(`consoleLog`) + } + }).waitForRouteChange() + + cy.get(`@consoleLog`).should(`be.calledWithMatch`, /Anonymous function declarations cause Fast Refresh to not preserve local component state./i) + }) + }) +} diff --git a/e2e-tests/development-runtime/src/pages/eslint-rules/limited-exports-page-templates.js b/e2e-tests/development-runtime/src/pages/eslint-rules/limited-exports-page-templates.js new file mode 100644 index 0000000000000..75a40504bb908 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/eslint-rules/limited-exports-page-templates.js @@ -0,0 +1,27 @@ +import React from "react" +import { graphql } from "gatsby" + +function PageQuery({ data }) { + return ( +
+

Limited Exports Page Templates. ESLint Rule

+

+ {data.site.siteMetadata.title} +

+
+ ) +} + +export function notAllowed() {} + +export const query = graphql` + { + site { + siteMetadata { + title + } + } + } +` + +export default PageQuery diff --git a/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates-function.js b/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates-function.js new file mode 100644 index 0000000000000..e7ef2e9fbd5a1 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates-function.js @@ -0,0 +1,11 @@ +import React from "react" + +import Layout from "../../components/layout" + +export default function () { + return ( + +

Anonymous Arrow Function. ESLint Rule

+ + ) +} diff --git a/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates.js b/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates.js new file mode 100644 index 0000000000000..4378c7bbcb324 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/eslint-rules/no-anonymous-exports-page-templates.js @@ -0,0 +1,9 @@ +import React from "react" + +import Layout from "../../components/layout" + +export default () => ( + +

Anonymous Arrow Function. ESLint Rule

+ +) diff --git a/packages/gatsby/src/utils/__tests__/eslint-config.ts b/packages/gatsby/src/utils/__tests__/eslint-config.ts new file mode 100644 index 0000000000000..e9fdf61f41822 --- /dev/null +++ b/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 [ + "/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 e5b5c64ca1f9a..0064ab2dd07f3 100644 --- a/packages/gatsby/src/utils/eslint-config.ts +++ b/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,13 +64,17 @@ export const eslintConfig = ( return { useEslintrc: false, resolvePluginsRelativeTo: __dirname, + 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: { // New versions of react use a special jsx runtime that remove the requirement @@ -32,7 +92,7 @@ export const eslintConfig = ( tagName: `graphql`, }, ], - "react/jsx-pascal-case": `off`, // Prevents errors with Theme-UI and Styled component + "react/jsx-pascal-case": `warn`, // https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/master/docs/rules "jsx-a11y/accessible-emoji": `warn`, "jsx-a11y/alt-text": `warn`, @@ -98,7 +158,7 @@ export const eslintConfig = ( ], }, ], - //"jsx-a11y/label-has-for": `warn`, was deprecated and replaced with jsx-a11y/has-associated-control in v6.1.0 + // "jsx-a11y/label-has-for": `warn`, was deprecated and replaced with jsx-a11y/has-associated-control in v6.1.0 "jsx-a11y/label-has-associated-control": `warn`, "jsx-a11y/lang": `warn`, "jsx-a11y/media-has-caption": `warn`, diff --git a/packages/gatsby/src/utils/eslint-rules-helpers.ts b/packages/gatsby/src/utils/eslint-rules-helpers.ts new file mode 100644 index 0000000000000..2cf1cd0f0fa3b --- /dev/null +++ b/packages/gatsby/src/utils/eslint-rules-helpers.ts @@ -0,0 +1,22 @@ +import { Rule } from "eslint" +import { GatsbyReduxStore } from "../redux" + +export function isPageTemplate( + s: GatsbyReduxStore, + c: Rule.RuleContext +): boolean { + const filename = c.getFilename() + if (!filename) { + return false + } + return s.getState().components.has(filename) +} + +export function test(t): any { + return Object.assign(t, { + parserOptions: { + sourceType: `module`, + ecmaVersion: 9, + }, + }) +} diff --git a/packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts b/packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts new file mode 100644 index 0000000000000..6859772206217 --- /dev/null +++ b/packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts @@ -0,0 +1,91 @@ +import { RuleTester } from "eslint" +import { test } from "../../eslint-rules-helpers" +const rule = require(`../limited-exports-page-templates`) + +const parserOptions = { + ecmaVersion: 2018, + sourceType: `module`, + ecmaFeatures: { + jsx: true, + }, +} + +const ruleTester = new RuleTester({ parserOptions }) + +jest.mock(`../../eslint-rules-helpers`, () => { + return { + ...jest.requireActual(`../../eslint-rules-helpers`), + isPageTemplate: jest.fn().mockReturnValue(true), + } +}) + +describe(`no-anonymous-exports-page-templates`, () => { + ruleTester.run(`passes valid and invalid cases`, rule, { + valid: [ + test({ + code: `import { graphql, Link } from "gatsby"\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`, + }), + test({ + code: `const { graphql } = require("gatsby")\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`, + }), + test({ + code: `const { graphql } = require(\`gatsby\`)\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`, + }), + test({ + code: `const { graphql: wat } = require("gatsby")\nconst Template = () => {}\nexport const query = wat\`test\`\nexport default Template`, + }), + test({ + code: `const { graphql: wat } = require(\`gatsby\`)\nconst Template = () => {}\nexport const query = wat\`test\`\nexport default Template`, + }), + test({ + code: `import { graphql as wat, Link } from "gatsby"\nconst Template = () => {}\nexport const query = wat\`test\`\nexport default Template`, + }), + test({ + code: `import * as Gatsby from "gatsby"\nconst Template = () => {}\nexport const query = Gatsby.graphql\`test\`\nexport default Template`, + }), + test({ + code: `const Template = () => {}\nexport default Template`, + }), + test({ + code: `import graphql from "graphql-tag"\nconst Template = () => {}\nconst stuff = graphql\`test\`\nexport default Template`, + }), + test({ + code: `import { graphql } from "gatsby"\nconst Template = () => {}\nconst query = graphql\`test\`\nexport { query }\nexport default Template`, + }), + ], + invalid: [ + test({ + code: `import { graphql } from "gatsby"\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport function Test() {}\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import graphql from "gatsby"\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import graphql from "graphql-tag"\nconst Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import { graphql as wat } from "gatsby"\nimport graphql from "graphql-tag"\nconst Template = () => {}\nexport const query = wat\`test\`\nexport const query2 = graphql\`test2\`\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import { graphql } from "gatsby"\nconst Template = () => {}\nconst query = graphql\`test\`\nfunction Test() {}\nexport { query, Test }\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import { graphql } from "gatsby"\nconst Template = () => {}\nexport const query = graphql\`test\`, hello = 10\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import * as Gatsby from "gatsby"\nconst Template = () => {}\nexport const query = Gatsby.graphql\`test\`, hello = 10\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + test({ + code: `import { graphql } from "gatsby"\nconst Template = () => {}\nexport const hello = 10, query = graphql\`test\`\nexport default Template`, + errors: [{ messageId: `limitedExportsPageTemplates` }], + }), + ], + }) +}) diff --git a/packages/gatsby/src/utils/eslint-rules/__tests__/no-anonymous-exports-page-templates.ts b/packages/gatsby/src/utils/eslint-rules/__tests__/no-anonymous-exports-page-templates.ts new file mode 100644 index 0000000000000..959790489ae53 --- /dev/null +++ b/packages/gatsby/src/utils/eslint-rules/__tests__/no-anonymous-exports-page-templates.ts @@ -0,0 +1,53 @@ +import { RuleTester } from "eslint" +import { test } from "../../eslint-rules-helpers" +const rule = require(`../no-anonymous-exports-page-templates`) + +const parserOptions = { + ecmaVersion: 2018, + sourceType: `module`, + ecmaFeatures: { + jsx: true, + }, +} + +const ruleTester = new RuleTester({ parserOptions }) + +jest.mock(`../../eslint-rules-helpers`, () => { + return { + ...jest.requireActual(`../../eslint-rules-helpers`), + isPageTemplate: jest.fn().mockReturnValue(true), + } +}) + +describe(`no-anonymous-exports-page-templates`, () => { + ruleTester.run(`passes valid and invalid cases`, rule, { + valid: [ + // Exports with identifiers are valid + test({ code: `const Named = () => {}\nexport default Named` }), + test({ code: `export default function foo() {}` }), + test({ code: `export default class MyClass {}` }), + + // Sanity check unrelated export syntaxes + test({ code: `export * from 'foo'` }), + test({ code: `const foo = 123\nexport { foo }` }), + test({ code: `const foo = 123\nexport { foo as default }` }), + + // Allow call expressions by default for backwards compatibility + test({ code: `export default foo(bar)` }), + ], + invalid: [ + test({ + code: `export default () => {}`, + errors: [{ messageId: `anonymousArrowFunction` }], + }), + test({ + code: `export default function() {}`, + errors: [{ messageId: `anonymousFunctionDeclaration` }], + }), + test({ + code: `export default class {}`, + errors: [{ messageId: `anonymousClass` }], + }), + ], + }) +}) diff --git a/packages/gatsby/src/utils/eslint-rules/limited-exports-page-templates.ts b/packages/gatsby/src/utils/eslint-rules/limited-exports-page-templates.ts new file mode 100644 index 0000000000000..9284bb7645e07 --- /dev/null +++ b/packages/gatsby/src/utils/eslint-rules/limited-exports-page-templates.ts @@ -0,0 +1,204 @@ +import { Rule } from "eslint" +import { + Node, + Identifier, + ImportDeclaration, + TaggedTemplateExpression, + VariableDeclaration, + CallExpression, + Literal, + TemplateLiteral, + VariableDeclarator, + ObjectPattern, +} from "estree" +import { store } from "../../redux" +import { isPageTemplate } from "../eslint-rules-helpers" + +const DEFAULT_GRAPHQL_TAG_NAME = `graphql` + +function hasOneValidNamedDeclaration( + node: Node, + varName: string | undefined +): boolean { + // Checks for: + // const query = graphql`` + // export { query } + if ( + node.type === `ExportNamedDeclaration` && + node.declaration === null && + varName + ) { + // For export { foobar, query } the declaration will be null and specifiers exists + // For { foobar, query } it'll return true, for { query } it'll return false + const nonQueryExports = node.specifiers.some( + e => e.exported.name !== varName + ) + return !nonQueryExports + } + + return false +} + +function isTemplateQuery( + node: Node, + graphqlTagName: string, + namespaceSpecifierName: string +): boolean { + // For export const query = 'foobar' the declaration exists with type 'VariableDeclaration' + + // Checks for: + // export const query = graphql`` + // This case only has one item in the declarations array + // For export const hello = 10, world = 'foo' + // The array will have two items. So use every() to check if only one item exists + // With TaggedTemplateExpression and "graphql" name + + // In addition the declaration can also be a MemberExpression like + // Gatsby.graphql`` when the import happened with import * as Gatsby from "gatsby" + + return ( + node.type === `ExportNamedDeclaration` && + node.declaration?.type === `VariableDeclaration` && + node.declaration?.declarations.every(el => { + if ( + el?.init?.type === `TaggedTemplateExpression` && + el.init.tag.type === `Identifier` + ) { + return el.init.tag.name === graphqlTagName + } else if ( + el?.init?.type === `TaggedTemplateExpression` && + el.init.tag.type === `MemberExpression` + ) { + return ( + (el.init.tag.object as Identifier).name === namespaceSpecifierName && + (el.init.tag.property as Identifier).name === DEFAULT_GRAPHQL_TAG_NAME + ) + } + return false + }) + ) +} + +const limitedExports: Rule.RuleModule = { + meta: { + type: `problem`, + messages: { + limitedExportsPageTemplates: `In page templates only a default export of a valid React component and the named export of a page query is allowed. + All other named exports will cause Fast Refresh to not preserve local component state and do a full refresh. + + Please move your other named exports to another file. Also make sure that you only export page queries that use the "graphql" tag from "gatsby". +`, + }, + }, + create: context => { + if (!isPageTemplate(store, context)) { + return {} + } + + let queryVariableName: string | undefined = `` + let graphqlTagName = `` + let namespaceSpecifierName = `` + + return { + // const { graphql } = require('gatsby') + VariableDeclaration: (node): void => { + // Check if require('gatsby') + const requiredFromGatsby = (node as VariableDeclaration).declarations.find( + el => { + // Handle require(`gatsby`) + if ( + (el.init as CallExpression)?.arguments?.[0]?.type === + `TemplateLiteral` + ) { + return ( + ((el.init as CallExpression).arguments[0] as TemplateLiteral) + ?.quasis[0].value.raw === `gatsby` + ) + } + + return ( + ((el.init as CallExpression)?.arguments?.[0] as Literal) + ?.value === `gatsby` + ) + } + ) + + if (requiredFromGatsby) { + // Search for "graphql" in a const { graphql, Link } = require('gatsby') + const graphqlTagSpecifier = ((requiredFromGatsby as VariableDeclarator) + .id as ObjectPattern)?.properties.find( + el => (el.key as Identifier).name === DEFAULT_GRAPHQL_TAG_NAME + ) + + if (graphqlTagSpecifier) { + graphqlTagName = (graphqlTagSpecifier.value as Identifier).name + } + } + + return undefined + }, + // import { graphql } from "gatsby" + ImportDeclaration: (node): void => { + // Make sure that the specifier is imported from "gatsby" + if ((node as ImportDeclaration).source.value === `gatsby`) { + const graphqlTagSpecifier = (node as ImportDeclaration).specifiers.find( + el => { + // We only want import { graphql } from "gatsby" + // Not import graphql from "gatsby" + if (el.type === `ImportSpecifier`) { + // Only get the specifier with the original name of "graphql" + return el.imported.name === DEFAULT_GRAPHQL_TAG_NAME + } + // import * as Gatsby from "gatsby" + if (el.type === `ImportNamespaceSpecifier`) { + namespaceSpecifierName = el.local.name + return false + } + return false + } + ) + if (graphqlTagSpecifier) { + // The local.name handles the case for import { graphql as otherName } + // For normal import { graphql } the imported & local name are the same + graphqlTagName = graphqlTagSpecifier.local.name + } + } + return undefined + }, + TaggedTemplateExpression: (node): void => { + if ( + (node as TaggedTemplateExpression).type === + `TaggedTemplateExpression` && + ((node as TaggedTemplateExpression).tag as Identifier)?.name === + graphqlTagName + ) { + if (queryVariableName) { + return undefined + } + // @ts-ignore + queryVariableName = node.parent?.id?.name + } + + return undefined + }, + ExportNamedDeclaration: (node): void => { + if (hasOneValidNamedDeclaration(node, queryVariableName)) { + return undefined + } + + if (isTemplateQuery(node, graphqlTagName, namespaceSpecifierName)) { + return undefined + } + + context.report({ + node, + messageId: `limitedExportsPageTemplates`, + }) + + return undefined + }, + } + }, +} + +module.exports = limitedExports diff --git a/packages/gatsby/src/utils/eslint-rules/no-anonymous-exports-page-templates.ts b/packages/gatsby/src/utils/eslint-rules/no-anonymous-exports-page-templates.ts new file mode 100644 index 0000000000000..8b130aed86abc --- /dev/null +++ b/packages/gatsby/src/utils/eslint-rules/no-anonymous-exports-page-templates.ts @@ -0,0 +1,79 @@ +import { Rule } from "eslint" +import { Node, ExportDefaultDeclaration } from "estree" +import { store } from "../../redux" +import { isPageTemplate } from "../eslint-rules-helpers" + +const defs = { + ArrowFunctionExpression: { + messageId: `anonymousArrowFunction`, + }, + FunctionDeclaration: { + messageId: `anonymousFunctionDeclaration`, + forbid: (node): boolean => !node.declaration.id, + }, + ClassDeclaration: { + messageId: `anonymousClass`, + forbid: (node): boolean => !node.declaration.id, + }, +} + +const noAnonymousExports: Rule.RuleModule = { + meta: { + type: `problem`, + messages: { + anonymousArrowFunction: `Anonymous arrow functions cause Fast Refresh to not preserve local component state. + + Please add a name to your function, for example: + + Before: + export default () => {} + + After: + const Named = () => {} + export default Named; +`, + anonymousFunctionDeclaration: `Anonymous function declarations cause Fast Refresh to not preserve local component state. + + Please add a name to your function, for example: + + Before: + export default function () {} + + After: + export default function Named() {} +`, + anonymousClass: `Anonymous classes cause Fast Refresh to not preserve local component state. + + Please add a name to your class, for example: + + Before: + export default class extends Component {} + + After: + export default class Named extends Component {} +`, + }, + }, + create: context => { + if (!isPageTemplate(store, context)) { + return {} + } + + return { + ExportDefaultDeclaration: (node: Node): void => { + // @ts-ignore + const type = node.declaration.type as ExportDefaultDeclaration["type"] + const def = defs[type] + + if (def && (!def.forbid || def.forbid(node))) { + context.report({ + node, + messageId: def.messageId, + }) + } + }, + } + }, +} + +module.exports = noAnonymousExports diff --git a/packages/gatsby/src/utils/eslint/required.js b/packages/gatsby/src/utils/eslint/required.js new file mode 100644 index 0000000000000..7ac22d57db856 --- /dev/null +++ b/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`, + }, +} diff --git a/packages/gatsby/src/utils/webpack-utils.ts b/packages/gatsby/src/utils/webpack-utils.ts index 77c942504f122..42255099e15b0 100644 --- a/packages/gatsby/src/utils/webpack-utils.ts +++ b/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 = (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 +} diff --git a/packages/gatsby/src/utils/webpack.config.js b/packages/gatsby/src/utils/webpack.config.js index 669d941995a28..9db7273c17770 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 } 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 }