feat(gatsby): Add eslint rules to warn against bad patterns in pageTemplates (for Fast Refresh)

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

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

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 (
+    <div>
+      <h1 data-testid="title">Limited Exports Page Templates. ESLint Rule</h1>
+      <p data-testid="hot">
+        {data.site.siteMetadata.title}
+      </p>
+    </div>
+  )
+}
+
+export function notAllowed() {}
+
+export const query = graphql`
+  {
+    site {
+      siteMetadata {
+        title
+      }
+    }
+  }
+`
+
+export default PageQuery

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 (
+    <Layout>
+      <h1 data-testid="title">Anonymous Arrow Function. ESLint Rule</h1>
+    </Layout>
+  )
+}

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 () => (
+  <Layout>
+    <h1 data-testid="title">Anonymous Arrow Function. ESLint Rule</h1>
+  </Layout>
+)

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

@@ -8,6 +8,7 @@ export const eslintConfig = (
   return {
     useEslintrc: false,
     resolvePluginsRelativeTo: __dirname,
+    rulePaths: [`${__dirname}/eslint-rules`],
     baseConfig: {
       globals: {
         graphql: true,
@@ -17,6 +18,9 @@ export const eslintConfig = (
       extends: [require.resolve(`eslint-config-react-app`)],
       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`.
@@ -32,7 +36,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 +102,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`,

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,
+    },
+  })
+}

packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts

@@ -0,0 +1,44 @@
+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),
+  }
+})
+
+ruleTester.run(`no-anonymous-exports-page-templates`, rule, {
+  valid: [
+    test({
+      code: `const Template = () => {}\nexport const query = graphql\`test\`\nexport default Template`,
+    }),
+    test({
+      code: `const Template = () => {}\nexport default Template`,
+    }),
+    test({
+      code: `const Template = () => {}\nconst query = graphql\`test\`\nexport { query }\nexport default Template`,
+    }),
+  ],
+  invalid: [
+    test({
+      code: `const Template = () => {}\nexport const query = graphql\`test\`\nexport function Test() {}\nexport default Template`,
+      errors: [{ messageId: `limitedExportsPageTemplates` }],
+    }),
+    test({
+      code: `const Template = () => {}\nconst query = graphql\`test\`\nfunction Test() {}\nexport { query, Test }\nexport default Template`,
+      errors: [{ messageId: `limitedExportsPageTemplates` }],
+    }),
+  ],
+})

packages/gatsby/src/utils/eslint-rules/__tests__/no-anonymous-exports-page-templates.ts

@@ -0,0 +1,47 @@
+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),
+  }
+})
+
+ruleTester.run(`no-anonymous-exports-page-templates`, 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` }],
+    }),
+  ],
+})

packages/gatsby/src/utils/eslint-rules/limited-exports-page-templates.ts

@@ -0,0 +1,80 @@
+import { Rule } from "eslint"
+import { Node, Identifier } from "estree"
+import { store } from "../../redux"
+import { isPageTemplate } from "../eslint-rules-helpers"
+
+function isTemplateQuery(node: Node, varName: string | undefined): boolean {
+  // Checks for:
+  // const query = graphql``
+  // export { query }
+  if (
+    node?.type === `ExportNamedDeclaration` &&
+    node?.declaration === null &&
+    varName
+  ) {
+    // For export { foobar } the declaration will be null and specifiers exists
+    const nonQueryExports = node.specifiers.find(
+      e => e.exported.name !== varName
+    )
+    return !nonQueryExports
+  } else {
+    // For export const query = 'foobar' the declaration exists with type 'VariableDeclaration'
+
+    // Checks for:
+    // export const query = graphql``
+    return (
+      node?.type === `ExportNamedDeclaration` &&
+      node?.declaration?.type === `VariableDeclaration` &&
+      node?.declaration?.declarations[0]?.init?.type ===
+        `TaggedTemplateExpression` &&
+      (node?.declaration?.declarations[0]?.init?.tag as Identifier)?.name ===
+        `graphql`
+    )
+  }
+}
+
+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.
+`,
+    },
+  },
+  create: context => {
+    if (!isPageTemplate(store, context)) {
+      return {}
+    }
+
+    let queryVariableName: string | undefined = ``
+
+    return {
+      TaggedTemplateExpression: (node): void => {
+        if (
+          node?.type === `TaggedTemplateExpression` &&
+          // @ts-ignore
+          node?.tag?.name === `graphql`
+        ) {
+          // @ts-ignore
+          queryVariableName = node?.parent?.id?.name
+        }
+      },
+      // eslint-disable-next-line consistent-return
+      ExportNamedDeclaration: (node): void => {
+        if (isTemplateQuery(node, queryVariableName)) {
+          return undefined
+        }
+
+        context.report({
+          node,
+          messageId: `limitedExportsPageTemplates`,
+        })
+      },
+    }
+  },
+}
+
+module.exports = limitedExports

packages/gatsby/src/utils/eslint-rules/no-anonymous-exports-page-templates.ts

@@ -0,0 +1,65 @@
+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,
+  },
+}
+
+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() {}
+`,
+    },
+  },
+  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