[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component (#27257)

awareness481 · web-flow · commit a28e775e8814 · 2021-08-13T23:16:15.000Z
## Feature - [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. [Feature Request](#26365) - [x] Eslint unit ests added - [x] Errors have helpful link attached, see `contributing.md` Let me know if this looks good or something needs to be changed. I still need to add the error links and improve the eslint error messages. I don't know if the CI runs the ESLint tests, but current all pass locally
diff --git a/errors/manifest.json b/errors/manifest.json
@@ -427,6 +427,14 @@
           "title": "sharp-missing-in-production",
           "path": "/errors/sharp-missing-in-production.md"
         },
+        {
+          "title": "script-in-document-page",
+          "path": "/errors/no-script-in-document-page.md"
+        },
+        {
+          "title": "script-in-head-component",
+          "path": "/errors/no-script-in-head-component.md"
+        },
         {
           "title": "max-custom-routes-reached",
           "path": "/errors/max-custom-routes-reached.md"
diff --git a/errors/no-script-in-document-page.md b/errors/no-script-in-document-page.md
@@ -0,0 +1,27 @@
+# Script component inside \_document.js
+
+#### Why This Error Occurred
+
+You can't use the `next/script` component inside the `_document.js` page. That's because the `_document.js` page only runs on the server and `next/script` has client-side functionality to ensure loading order.
+
+#### Possible Ways to Fix It
+
+If you want a global script, instead use the `_app.js` page.
+
+```jsx
+import Script from 'next/script'
+
+function MyApp({ Component, pageProps }) {
+  return (
+    <>
+      <Script src="/my-script.js" />
+      <Component {...pageProps} />
+    </>
+  )
+}
+
+export default MyApp
+```
+
+- [custom-app](https://nextjs.org/docs/advanced-features/custom-app)
+- [next-script](https://nextjs.org/docs/basic-features/script#usage)
diff --git a/errors/no-script-in-head-component.md b/errors/no-script-in-head-component.md
@@ -0,0 +1,48 @@
+# Script component inside Head component
+
+#### Why This Error Occurred
+
+The `next/script` component shouldn't be placed inside the `next/head` component
+
+#### Possible Ways to Fix It
+
+Move the `<Script />` component outside of `<Head>...</Head>`
+
+**Before**
+
+```js
+import Script from 'next/script'
+import Head from 'next/head'
+
+export default function Index() {
+  return (
+    <Head>
+      <title>Next.js</title>
+      <Script src="/my-script.js" />
+    </Head>
+  )
+}
+```
+
+**After**
+
+```js
+import Script from 'next/script'
+import Head from 'next/head'
+
+export default function Index() {
+  return (
+    <>
+      <Head>
+        <title>Next.js</title>
+      </Head>
+      <Script src="/my-script.js" />
+    </>
+  )
+}
+```
+
+### Useful links
+
+- [next/head](https://nextjs.org/docs/api-reference/next/head)
+- [next/script](https://nextjs.org/docs/basic-features/script#usage)
diff --git a/packages/eslint-plugin-next/lib/index.js b/packages/eslint-plugin-next/lib/index.js
@@ -12,6 +12,8 @@ module.exports = {
     'link-passhref': require('./rules/link-passhref'),
     'no-document-import-in-page': require('./rules/no-document-import-in-page'),
     'no-head-import-in-document': require('./rules/no-head-import-in-document'),
+    'no-script-in-document': require('./rules/no-script-in-document'),
+    'no-script-in-head': require('./rules/no-script-in-head'),
     'no-typos': require('./rules/no-typos'),
     'no-duplicate-head': require('./rules/no-duplicate-head'),
   },
@@ -31,6 +33,8 @@ module.exports = {
         '@next/next/link-passhref': 1,
         '@next/next/no-document-import-in-page': 2,
         '@next/next/no-head-import-in-document': 2,
+        '@next/next/no-script-in-document': 2,
+        '@next/next/no-script-in-head': 2,
         '@next/next/no-typos': 1,
         '@next/next/no-duplicate-head': 2,
       },
diff --git a/packages/eslint-plugin-next/lib/rules/no-script-in-document.js b/packages/eslint-plugin-next/lib/rules/no-script-in-document.js
@@ -0,0 +1,29 @@
+const path = require('path')
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Disallow importing next/script inside pages/_document.js',
+      recommended: true,
+    },
+  },
+  create: function (context) {
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value !== 'next/script') {
+          return
+        }
+
+        const document = context.getFilename().split('pages')[1]
+        if (!document || !path.parse(document).name.startsWith('_document')) {
+          return
+        }
+
+        context.report({
+          node,
+          message: `next/script should not be used in pages/_document.js. See: https://nextjs.org/docs/messages/no-script-in-document-page `,
+        })
+      },
+    }
+  },
+}
diff --git a/packages/eslint-plugin-next/lib/rules/no-script-in-head.js b/packages/eslint-plugin-next/lib/rules/no-script-in-head.js
@@ -0,0 +1,51 @@
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Disallow using next/script inside the next/head component',
+      recommended: true,
+    },
+  },
+  create: function (context) {
+    let isNextHead = null
+
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value === 'next/head') {
+          isNextHead = node.source.value
+        }
+
+        if (node.source.value !== 'next/script') {
+          return
+        }
+      },
+      JSXElement(node) {
+        if (!isNextHead) {
+          return
+        }
+
+        if (
+          node.openingElement &&
+          node.openingElement.name &&
+          node.openingElement.name.name !== 'Head'
+        ) {
+          return
+        }
+
+        const scriptTag = node.children.find(
+          (child) =>
+            child.openingElement &&
+            child.openingElement.name &&
+            child.openingElement.name.name === 'Script'
+        )
+
+        if (scriptTag) {
+          context.report({
+            node,
+            message:
+              "next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component ",
+          })
+        }
+      },
+    }
+  },
+}
diff --git a/test/eslint-plugin-next/no-script-in-document.test.js b/test/eslint-plugin-next/no-script-in-document.test.js
@@ -0,0 +1,117 @@
+const rule = require('@next/eslint-plugin-next/lib/rules/no-script-in-document')
+
+const RuleTester = require('eslint').RuleTester
+
+RuleTester.setDefaultConfig({
+  parserOptions: {
+    ecmaVersion: 2018,
+    sourceType: 'module',
+    ecmaFeatures: {
+      modules: true,
+      jsx: true,
+    },
+  },
+})
+
+var ruleTester = new RuleTester()
+ruleTester.run('no-script-import-in-document', rule, {
+  valid: [
+    {
+      code: `import Document, { Html, Head, Main, NextScript } from 'next/document'
+
+      class MyDocument extends Document {
+        static async getInitialProps(ctx) {
+          //...
+        }
+      
+        render() {
+          return (
+            <Html>
+              <Head/>
+            </Html>
+          )
+        }
+      }
+      
+      export default MyDocument
+    `,
+      filename: 'pages/_document.js',
+    },
+    {
+      code: `import Document, { Html, Head, Main, NextScript } from 'next/document'
+
+      class MyDocument extends Document {      
+        render() {
+          return (
+            <Html>
+              <Head>
+                <meta charSet="utf-8" />
+              </Head>
+            </Html>
+          )
+        }
+      }
+      
+      export default MyDocument
+    `,
+      filename: 'pages/_document.tsx',
+    },
+  ],
+  invalid: [
+    {
+      code: `
+      import Document, { Html, Main, NextScript } from 'next/document'
+      import Script from 'next/script'
+      
+      class MyDocument extends Document {
+        render() {
+          return (
+            <Html>
+              <Head />
+            </Html>
+          )
+        }
+      }
+      
+      export default MyDocument
+      `,
+      filename: 'pages/_document.js',
+      errors: [
+        {
+          message: `next/script should not be used in pages/_document.js. See: https://nextjs.org/docs/messages/no-script-in-document-page `,
+        },
+      ],
+    },
+    {
+      code: `
+      import Document, { Html, Main, NextScript } from 'next/document'
+      import NextScriptTag from 'next/script'
+      
+      class MyDocument extends Document {
+        render() {
+          return (
+            <Html>
+              <Head>
+                <meta charSet="utf-8" />
+              </Head>
+              <body>
+                <Main />
+                <NextScript />
+                <NextScriptTag />
+              </body>
+            </Html>
+          )
+        }
+      }
+      
+      export default MyDocument
+      `,
+      filename: 'pages/_document.js',
+      errors: [
+        {
+          message: `next/script should not be used in pages/_document.js. See: https://nextjs.org/docs/messages/no-script-in-document-page `,
+        },
+      ],
+    },
+  ],
+})
diff --git a/test/eslint-plugin-next/no-script-in-head.test.js b/test/eslint-plugin-next/no-script-in-head.test.js
@@ -0,0 +1,53 @@
+const rule = require('@next/eslint-plugin-next/lib/rules/no-script-in-head.js')
+const RuleTester = require('eslint').RuleTester
+
+RuleTester.setDefaultConfig({
+  parserOptions: {
+    ecmaVersion: 2018,
+    sourceType: 'module',
+    ecmaFeatures: {
+      modules: true,
+      jsx: true,
+    },
+  },
+})
+
+var ruleTester = new RuleTester()
+ruleTester.run('no-script-in-head', rule, {
+  valid: [
+    `import Script from "next/script";
+     const Head = ({children}) => children
+
+    export default function Index() {
+      return (
+        <Head>
+          <Script></Script>
+        </Head>
+      );
+    }
+    `,
+  ],
+
+  invalid: [
+    {
+      code: `
+      import Head from "next/head";
+      import Script from "next/script";
+      
+      export default function Index() {
+        return (
+            <Head>
+              <Script></Script>
+            </Head>
+        );
+      }`,
+      filename: 'pages/index.js',
+      errors: [
+        {
+          message:
+            "next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component ",
+        },
+      ],
+    },
+  ],
+})
