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.

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

Warn in dev mode when script tags are added with next/head #33968

Merged
merged 2 commits into from Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 6 additions & 2 deletions errors/manifest.json
Expand Up @@ -481,8 +481,12 @@
"path": "/errors/no-script-in-document-page.md"
},
{
"title": "script-in-head-component",
"path": "/errors/no-script-in-head-component.md"
"title": "script-component-in-head-component",
"path": "/errors/no-script-component-in-head-component.md"
},
{
"title": "script-tags-in-head-component",
"path": "/errors/no-script-tags-in-head-component.md"
},
{
"title": "max-custom-routes-reached",
Expand Down
36 changes: 36 additions & 0 deletions errors/no-script-tags-in-head-component.md
@@ -0,0 +1,36 @@
# No Script Tags In Head Component

### Why This Error Occurred

A `<script>` tag was added using the `next/head` component.

We don't recommend this pattern because it will potentially break when used with Suspense and/or streaming. In these contexts, `next/head` tags aren't:

- guaranteed to be included in the initial SSR response, so loading could be delayed until client-side rendering, regressing performance.

- loaded in any particular order. The order that the app's Suspense boundaries resolve will determine the loading order of your scripts.

### Possible Ways to Fix It

#### Script component

Use the Script component with the right loading strategy to defer loading of the script until necessary. You can see the possible strategies [here](https://nextjs.org/docs/basic-features/script/.)

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

const Home = () => {
return (
<div class="container">
<Script src="https://third-party-script.js"></Script>
<div>Home Page</div>
</div>
)
}

export default Home
```

### Useful Links

- [Script component docs](https://nextjs.org/docs/basic-features/script/)
4 changes: 2 additions & 2 deletions packages/eslint-plugin-next/lib/index.js
Expand Up @@ -14,7 +14,7 @@ module.exports = {
'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-script-component-in-head': require('./rules/no-script-component-in-head'),
'no-server-import-in-page': require('./rules/no-server-import-in-page'),
'no-typos': require('./rules/no-typos'),
'no-duplicate-head': require('./rules/no-duplicate-head'),
Expand All @@ -40,7 +40,7 @@ module.exports = {
'@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-script-component-in-head': 2,
'@next/next/no-server-import-in-page': 2,
'@next/next/no-typos': 1,
'@next/next/no-duplicate-head': 2,
Expand Down
Expand Up @@ -3,7 +3,7 @@ module.exports = {
docs: {
description: 'Disallow using next/script inside the next/head component',
recommended: true,
url: 'https://nextjs.org/docs/messages/no-script-in-head-component',
url: 'https://nextjs.org/docs/messages/no-script-component-in-head-component',
},
},
create: function (context) {
Expand Down Expand Up @@ -43,7 +43,7 @@ module.exports = {
context.report({
node,
message:
"next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component",
"next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-component-in-head-component",
})
}
},
Expand Down
6 changes: 6 additions & 0 deletions packages/next/shared/lib/head.tsx
Expand Up @@ -161,6 +161,12 @@ function reduceComponents(
return React.cloneElement(c, newProps)
}
}
// TODO(kara): warn for stylesheets as well as scripts
if (process.env.NODE_ENV === 'development' && c.type === 'script') {
console.warn(
`Do not add <script> tags using next/head. Use next/script instead. \nSee more info here: https://nextjs.org/docs/messages/no-script-tags-in-head-component`
)
}
return React.cloneElement(c, { key })
})
}
Expand Down
23 changes: 23 additions & 0 deletions test/integration/client-navigation/test/index.test.js
Expand Up @@ -1391,6 +1391,29 @@ describe('Client Navigation', () => {
}
})

it('should warn when scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head')

await browser.waitForElementByCss('h1')
await waitFor(2000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
console.log('log.message', log.message)
if (log.message.includes('Use next/script instead')) {
found = true
}
})
expect(found).toEqual(true)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should update head during client routing', async () => {
let browser
try {
Expand Down
@@ -1,4 +1,4 @@
import rule from '@next/eslint-plugin-next/lib/rules/no-script-in-head'
import rule from '@next/eslint-plugin-next/lib/rules/no-script-component-in-head'
import { RuleTester } from 'eslint'
;(RuleTester as any).setDefaultConfig({
parserOptions: {
Expand Down Expand Up @@ -44,7 +44,7 @@ ruleTester.run('no-script-in-head', rule, {
errors: [
{
message:
"next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component",
"next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-component-in-head-component",
},
],
},
Expand Down