Skip to content

Commit

Permalink
Add eslint rule for id attribute on inline next/script (#27853)
Browse files Browse the repository at this point in the history
This adds a new ESLint rule to `eslint-plugin-next` to check that `next/script` components with inline content have the required `id` attribute.

Also adjusted the code example for inline scripts in the `next/script` docs, which were actually missing an `id` attribute.
And also updated the `next/scripts` integration test to also have the required `id` attribute.

Unsure about the required heading levels in the errors .md document (other examples have h1 and h4??)

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [x] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes
  • Loading branch information
stefanprobst committed Aug 19, 2021
1 parent 52b858a commit da4203d
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 12 deletions.
1 change: 1 addition & 0 deletions docs/basic-features/eslint.md
Expand Up @@ -94,6 +94,7 @@ Next.js provides an ESLint plugin, [`eslint-plugin-next`](https://www.npmjs.com/
| ✔️ | [next/no-sync-scripts](https://nextjs.org/docs/messages/no-sync-scripts) | Forbid synchronous scripts |
| ✔️ | [next/no-title-in-document-head](https://nextjs.org/docs/messages/no-title-in-document-head) | Disallow using <title> with Head from next/document |
| ✔️ | [next/no-unwanted-polyfillio](https://nextjs.org/docs/messages/no-unwanted-polyfillio) | Prevent duplicate polyfills from Polyfill.io |
| ✔️ | [next/inline-script-id](https://nextjs.org/docs/messages/inline-script-id) | Enforce id attribute on next/script components with inline content |
| ✔️ | next/no-typos | Ensure no typos were made declaring [Next.js's data fetching function](https://nextjs.org/docs/basic-features/data-fetching) |
| ✔️ | [next/next-script-for-ga](https://nextjs.org/docs/messages/next-script-for-ga) | Use the Script component to defer loading of the script until necessary. |

Expand Down
3 changes: 2 additions & 1 deletion docs/basic-features/script.md
Expand Up @@ -131,13 +131,14 @@ export default function Home() {
```js
import Script from 'next/script'

<Script strategy="lazyOnload">
<Script id="show-banner" strategy="lazyOnload">
{`document.getElementById('banner').removeClass('hidden')`}
</Script>

// or

<Script
id="show-banner"
dangerouslySetInnerHTML={{
__html: `document.getElementById('banner').removeClass('hidden')`
}}
Expand Down
26 changes: 26 additions & 0 deletions errors/inline-script-id.md
@@ -0,0 +1,26 @@
# next/script components with inline content require an `id` attribute

## Why This Error Occurred

`next/script` components with inline content require an `id` attribute to be defined to track and optimize the script.

## Possible Ways to Fix It

Add an `id` attribute to the `next/script` component.

```jsx
import Script from 'next/script'

export default function App({ Component, pageProps }) {
return (
<>
<Script id="my-script">{`console.log('Hello world!');`}</Script>
<Component {...pageProps} />
</>
)
}
```

## Useful links

- [Docs for Next.js Script component](https://nextjs.org/docs/basic-features/script)
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -145,6 +145,10 @@
"title": "incompatible-href-as",
"path": "/errors/incompatible-href-as.md"
},
{
"title": "inline-script-id",
"path": "/errors/inline-script-id.md"
},
{ "title": "install-sass", "path": "/errors/install-sass.md" },
{ "title": "install-sharp", "path": "/errors/install-sharp.md" },
{
Expand Down
10 changes: 5 additions & 5 deletions errors/next-script-for-ga.md
Expand Up @@ -20,8 +20,8 @@ const Home = () => {
<Script
src="https://www.googletagmanager.com/gtag/js?id=GA_MEASUREMENT_ID"
strategy="lazyOnload"
></Script>
<Script>
/>
<Script id="google-analytics">
{`
window.dataLayer = window.dataLayer || [];
function gtag(){window.dataLayer.push(arguments);}
Expand All @@ -47,7 +47,7 @@ import Script from 'next/script'
const Home = () => {
return (
<div class="container">
<Script>
<Script id="google-analytics">
{`
(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
Expand All @@ -73,7 +73,7 @@ import Script from 'next/script'
const Home = () => {
return (
<div class="container">
<Script>
<Script id="google-analytics">
{`
window.ga=window.ga||function(){(ga.q=ga.q||[]).push(arguments)};ga.l=+new Date;
ga('create', 'GOOGLE_ANALYTICS_ID', 'auto');
Expand All @@ -83,7 +83,7 @@ const Home = () => {
<Script
src="https://www.google-analytics.com/analytics.js"
strategy="lazyOnload"
></Script>
/>
</div>
)
}
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-next/lib/index.js
Expand Up @@ -16,6 +16,7 @@ module.exports = {
'no-script-in-head': require('./rules/no-script-in-head'),
'no-typos': require('./rules/no-typos'),
'no-duplicate-head': require('./rules/no-duplicate-head'),
'inline-script-id': require('./rules/inline-script-id'),
'next-script-for-ga': require('./rules/next-script-for-ga'),
},
configs: {
Expand All @@ -39,6 +40,7 @@ module.exports = {
'@next/next/no-script-in-head': 2,
'@next/next/no-typos': 1,
'@next/next/no-duplicate-head': 2,
'@next/next/inline-script-id': 2,
},
},
'core-web-vitals': {
Expand Down
48 changes: 48 additions & 0 deletions packages/eslint-plugin-next/lib/rules/inline-script-id.js
@@ -0,0 +1,48 @@
module.exports = {
meta: {
docs: {
description:
'next/script components with inline content must specify an `id` attribute.',
recommended: true,
},
},
create: function (context) {
let nextScriptImportName = null

return {
ImportDeclaration(node) {
if (node.source.value === 'next/script') {
nextScriptImportName = node.specifiers[0].local.name
}
},
JSXElement(node) {
if (nextScriptImportName == null) return

if (
node.openingElement &&
node.openingElement.name &&
node.openingElement.name.name !== nextScriptImportName
) {
return
}

const attributes = node.openingElement.attributes

if (
node.children.length > 0 ||
attributes.some(
(attribute) => attribute.name.name === 'dangerouslySetInnerHTML'
)
) {
if (!attributes.some((attribute) => attribute.name.name === 'id')) {
context.report({
node,
message:
'next/script components with inline content must specify an `id` attribute. See: https://nextjs.org/docs/messages/inline-script-id',
})
}
}
},
}
},
}
156 changes: 156 additions & 0 deletions test/eslint-plugin-next/inline-script-id.unit.test.js
@@ -0,0 +1,156 @@
const rule = require('@next/eslint-plugin-next/lib/rules/inline-script-id')

const RuleTester = require('eslint').RuleTester

RuleTester.setDefaultConfig({
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
modules: true,
jsx: true,
},
},
})

const errorMessage =
'next/script components with inline content must specify an `id` attribute. See: https://nextjs.org/docs/messages/inline-script-id'

const ruleTester = new RuleTester()
ruleTester.run('inline-script-id', rule, {
valid: [
{
code: `import Script from 'next/script';
export default function TestPage() {
return (
<Script id="test-script">
{\`console.log('Hello world');\`}
</Script>
)
}`,
},
{
code: `import Script from 'next/script';
export default function TestPage() {
return (
<Script
id="test-script"
dangerouslySetInnerHTML={{
__html: \`console.log('Hello world');\`
}}
/>
)
}`,
},
{
code: `import Script from 'next/script';
export default function TestPage() {
return (
<Script src="https://example.com" />
)
}`,
},
{
code: `import MyScript from 'next/script';
export default function TestPage() {
return (
<MyScript id="test-script">
{\`console.log('Hello world');\`}
</MyScript>
)
}`,
},
{
code: `import MyScript from 'next/script';
export default function TestPage() {
return (
<MyScript
id="test-script"
dangerouslySetInnerHTML={{
__html: \`console.log('Hello world');\`
}}
/>
)
}`,
},
],
invalid: [
{
code: `import Script from 'next/script';
export default function TestPage() {
return (
<Script>
{\`console.log('Hello world');\`}
</Script>
)
}`,
errors: [
{
message: errorMessage,
type: 'JSXElement',
},
],
},
{
code: `import Script from 'next/script';
export default function TestPage() {
return (
<Script
dangerouslySetInnerHTML={{
__html: \`console.log('Hello world');\`
}}
/>
)
}`,
errors: [
{
message: errorMessage,
type: 'JSXElement',
},
],
},
{
code: `import MyScript from 'next/script';
export default function TestPage() {
return (
<MyScript>
{\`console.log('Hello world');\`}
</MyScript>
)
}`,
errors: [
{
message: errorMessage,
type: 'JSXElement',
},
],
},
{
code: `import MyScript from 'next/script';
export default function TestPage() {
return (
<MyScript
dangerouslySetInnerHTML={{
__html: \`console.log('Hello world');\`
}}
/>
)
}`,
errors: [
{
message: errorMessage,
type: 'JSXElement',
},
],
},
],
})
10 changes: 5 additions & 5 deletions test/eslint-plugin-next/next-script-for-ga.unit.test.js
Expand Up @@ -29,8 +29,8 @@ ruleTester.run('sync-scripts', rule, {
<Script
src="https://www.googletagmanager.com/gtag/js?id=GA_MEASUREMENT_ID"
strategy="lazyOnload"
></Script>
<Script>
/>
<Script id="google-analytics">
{\`
window.dataLayer = window.dataLayer || [];
function gtag(){window.dataLayer.push(arguments);}
Expand All @@ -50,12 +50,12 @@ ruleTester.run('sync-scripts', rule, {
return (
<div>
<h1>Hello title</h1>
<Script>
<Script id="google-analytics">
{\`(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
})(window,document,'script','https://www.google-analytics.com/analytics.js','ga');
ga('create', 'UA-XXXXX-Y', 'auto');
ga('send', 'pageview');
})\`}
Expand All @@ -71,7 +71,7 @@ ruleTester.run('sync-scripts', rule, {
return (
<div>
<h1>Hello title</h1>
<Script>
<Script id="google-analytics">
{\`window.ga=window.ga||function(){(ga.q=ga.q||[]).push(arguments)};ga.l=+new Date;
ga('create', 'UA-XXXXX-Y', 'auto');
ga('send', 'pageview');
Expand Down
2 changes: 1 addition & 1 deletion test/integration/script-loader/pages/page3.js
Expand Up @@ -3,7 +3,7 @@ import Script from 'next/script'
const Page = () => {
return (
<div class="container">
<Script>
<Script id="inline-script">
{`(window.onload = function () {
const newDiv = document.createElement('div')
newDiv.id = 'onload-div'
Expand Down

0 comments on commit da4203d

Please sign in to comment.