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

This commit adds a development mode warning in the console
if you try to include <script> tags in next/head, e.g.

```
<Head>
  <script async src="..." />
</Head>
```

The warning message explains that this pattern will not
work well with Suspense/streaming and recommends using the
next/script component instead.

TODO in follow-up PR: add same warning for stylesheets, etc

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Integration tests added
- [x] Documentation added
- [x] Errors have helpful link attached, see `contributing.md`
This commit is contained in:
Kara 2022-02-04 05:06:55 -08:00 committed by GitHub
parent 06b263021a
commit 61ea8efe42
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 77 additions and 8 deletions

View file

@ -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",

View file

@ -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/)

View file

@ -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'),
@ -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,

View file

@ -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) {
@ -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",
})
}
},

View file

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

View file

@ -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 {

View file

@ -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: {
@ -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",
},
],
},