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

Changes to the beforeInteractive strategy to make it work for streaming #31936

Merged
merged 53 commits into from Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
bd0e593
Changes to the beforeInteractive strategy to make it work for streaming
janicklas-ralph Nov 29, 2021
74c3b90
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Jan 31, 2022
534cdc9
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 17, 2022
5f4b871
Updating design and adding eslint rule
janicklas-ralph Feb 17, 2022
34b1dfd
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 17, 2022
fb0c0c1
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 22, 2022
4183720
updating docs
janicklas-ralph Feb 22, 2022
375a603
fix review comments
janicklas-ralph Feb 23, 2022
60e0b6f
fix manifest.json
janicklas-ralph Feb 23, 2022
7830ead
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 23, 2022
162c6b2
fix manifest.json
janicklas-ralph Feb 23, 2022
b9af011
Fix review comments
janicklas-ralph Feb 24, 2022
1f0586d
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 24, 2022
899b506
Fix test case
janicklas-ralph Feb 25, 2022
0e2437b
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 25, 2022
a0f97b4
Fix test case
janicklas-ralph Feb 25, 2022
0a38ef9
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Feb 26, 2022
def82bc
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 4, 2022
5a0aaf8
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 5, 2022
caa62ca
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 14, 2022
b9d293f
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 18, 2022
1b3389b
Fix test case
janicklas-ralph Mar 18, 2022
bc9c136
Fix error in reder.tsx
janicklas-ralph Mar 18, 2022
d6967f9
Revert "Fix error in reder.tsx"
janicklas-ralph Mar 18, 2022
6530d68
Fix error in reder.tsx
janicklas-ralph Mar 18, 2022
15ea355
Fix error in reder.tsx
janicklas-ralph Mar 18, 2022
d83e4c5
Fix lint error when strategy is undefined in Script
janicklas-ralph Mar 18, 2022
73c282a
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 21, 2022
9660ff9
add error page back for users on older version of script
janicklas-ralph Mar 25, 2022
242324d
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Mar 25, 2022
7a591e5
Add error page back in manifest json
janicklas-ralph Mar 25, 2022
b370a52
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 5, 2022
99dcc1d
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 7, 2022
b6cb974
Resolve comments. Add tests for older beforeinteractive
janicklas-ralph Apr 7, 2022
450ab3b
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 7, 2022
316f368
Fix failing test
janicklas-ralph Apr 7, 2022
11fdcad
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 7, 2022
6627015
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 8, 2022
19f9ae1
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 8, 2022
1bec0e6
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 11, 2022
c6e2f2f
Fix failing test
janicklas-ralph Apr 11, 2022
d8d6a4a
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 11, 2022
01346c1
Fix failing test
janicklas-ralph Apr 11, 2022
beeb03a
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 11, 2022
243b0ef
Fix failing test
janicklas-ralph Apr 12, 2022
e00ed11
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 12, 2022
2bc903b
Add _document imports in examples
janicklas-ralph Apr 12, 2022
04253eb
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 12, 2022
0d8e9d5
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 20, 2022
e17be39
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 21, 2022
bf11680
Merge branch 'canary' of github.com:vercel/next.js into script-update
janicklas-ralph Apr 21, 2022
265c654
Add unstable_ prefix to the page scriptLoader
janicklas-ralph Apr 21, 2022
d292f25
Merge branch 'canary' into script-update
kodiakhq[bot] Apr 21, 2022
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
27 changes: 22 additions & 5 deletions docs/basic-features/script.md
Expand Up @@ -73,13 +73,30 @@ There are three different loading strategies that can be used:

#### beforeInteractive

Scripts that load with the `beforeInteractive` strategy are injected into the initial HTML from the server and run before self-bundled JavaScript is executed. This strategy should be used for any critical scripts that need to be fetched and executed before the page is interactive.
Scripts that load with the `beforeInteractive` strategy are injected into the initial HTML from the server and run before self-bundled JavaScript is executed. This strategy should be used for any critical scripts that need to be fetched and executed before any page becomes interactive. This strategy only works inside **\_document.js** and is designed to load scripts that are needed by the entire site (i.e. the script will load when any page in the application has been loaded server-side).

The reason `beforeInteractive` was designed to work only inside `\_document.js` is to support streaming and Suspense functionality. Outside of the `_document`, it's not possible to guarantee the timing or ordering of `beforeInteractive` scripts.

```jsx
<Script
src="https://cdn.jsdelivr.net/npm/cookieconsent@3/build/cookieconsent.min.js"
strategy="beforeInteractive"
/>
// In _document.js
import { Html, Head, Main, NextScript } from 'next/document'
import Script from 'next/script'

ijjk marked this conversation as resolved.
Show resolved Hide resolved
export default function Document() {
return (
<Html>
<Head />
<body>
<Main />
<NextScript />
<Script
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js"
strategy="beforeInteractive"
></Script>
</body>
</Html>
)
}
```

Examples of scripts that should be loaded as soon as possible with this strategy include:
Expand Down
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -562,6 +562,10 @@
"title": "script-in-document-page",
"path": "/errors/no-script-in-document-page.md"
},
{
"title": "before-interactive-script-outside-document",
"path": "/errors/no-before-interactive-script-outside-document.md"
},
{
"title": "script-component-in-head-component",
"path": "/errors/no-script-component-in-head-component.md"
Expand Down
33 changes: 33 additions & 0 deletions errors/no-before-interactive-script-outside-document.md
@@ -0,0 +1,33 @@
# beforeInteractive Script component outside \_document.js

#### Why This Error Occurred

You can't use the `next/script` component with the `beforeInteractive` strategy outside the `_document.js` page. That's because `beforeInteractive` strategy only works inside **\_document.js** and is designed to load scripts that are needed by the entire site (i.e. the script will load when any page in the application has been loaded server-side).

#### Possible Ways to Fix It

If you want a global script, move the script inside `_document.js` page.

```jsx
// In _document.js
import { Html, Head, Main, NextScript } from 'next/document'
import Script from 'next/script'

ijjk marked this conversation as resolved.
Show resolved Hide resolved
export default function Document() {
return (
<Html>
<Head />
<body>
<Main />
<NextScript />
<Script
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js"
strategy="beforeInteractive"
></Script>
</body>
</Html>
)
}
```

- [next-script](https://nextjs.org/docs/basic-features/script#usage)
4 changes: 2 additions & 2 deletions packages/eslint-plugin-next/lib/index.js
Expand Up @@ -13,13 +13,13 @@ 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-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'),
'inline-script-id': require('./rules/inline-script-id'),
'next-script-for-ga': require('./rules/next-script-for-ga'),
'no-before-interactive-script-outside-document': require('./rules/no-before-interactive-script-outside-document'),
'no-assign-module-variable': require('./rules/no-assign-module-variable'),
},
configs: {
Expand All @@ -40,12 +40,12 @@ module.exports = {
'@next/next/next-script-for-ga': 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-component-in-head': 2,
'@next/next/no-server-import-in-page': 2,
'@next/next/no-typos': 1,
'@next/next/no-duplicate-head': 2,
'@next/next/inline-script-id': 2,
'@next/next/no-before-interactive-script-outside-document': 1,
'@next/next/no-assign-module-variable': 2,
},
},
Expand Down
@@ -0,0 +1,51 @@
const path = require('path')

module.exports = {
meta: {
docs: {
description:
'Disallow using next/script beforeInteractive strategy outside the next/_document component',
recommended: true,
url: 'https://nextjs.org/docs/messages/no-before-interactive-script-outside-document',
},
},
create: function (context) {
let scriptImportName = null

return {
'ImportDeclaration[source.value="next/script"] > ImportDefaultSpecifier'(
node
) {
scriptImportName = node.local.name
},
JSXOpeningElement(node) {
if (!scriptImportName) {
return
}

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

const strategy = node.attributes.find(
(child) => child.name && child.name.name === 'strategy'
)

if (!strategy || strategy?.value?.value !== 'beforeInteractive') {
return
}

const document = context.getFilename().split('pages')[1]
if (document && path.parse(document).name.startsWith('_document')) {
return
}

context.report({
node,
message:
'next/script beforeInteractive strategy should only be used inside next/_document. See: https://nextjs.org/docs/messages/no-before-interactive-script-outside-document',
})
},
}
},
}
30 changes: 0 additions & 30 deletions packages/eslint-plugin-next/lib/rules/no-script-in-document.js

This file was deleted.

20 changes: 16 additions & 4 deletions packages/next/client/script.tsx
Expand Up @@ -108,14 +108,14 @@ const loadScript = (props: ScriptProps): void => {
document.body.appendChild(el)
}

function handleClientScriptLoad(props: ScriptProps) {
export function handleClientScriptLoad(props: ScriptProps) {
const { strategy = 'afterInteractive' } = props
if (strategy === 'afterInteractive') {
loadScript(props)
} else if (strategy === 'lazyOnload') {
if (strategy === 'lazyOnload') {
window.addEventListener('load', () => {
requestIdleCallback(() => loadScript(props))
})
} else {
loadScript(props)
}
}

Expand All @@ -129,8 +129,20 @@ function loadLazyScript(props: ScriptProps) {
}
}

function addBeforeInteractiveToCache() {
const scripts = [
...document.querySelectorAll('[data-nscript="beforeInteractive"]'),
...document.querySelectorAll('[data-nscript="beforePageRender"]'),
]
scripts.forEach((script) => {
const cacheKey = script.id || script.getAttribute('src')
LoadCache.add(cacheKey)
})
}

export function initScriptLoader(scriptLoaderItems: ScriptProps[]) {
scriptLoaderItems.forEach(handleClientScriptLoad)
addBeforeInteractiveToCache()
}

function Script(props: ScriptProps): JSX.Element | null {
Expand Down
11 changes: 10 additions & 1 deletion packages/next/server/render.tsx
Expand Up @@ -586,6 +586,7 @@ export async function renderToHTML(
App.getInitialProps === (App as any).origGetInitialProps

const hasPageGetInitialProps = !!(Component as any)?.getInitialProps
const hasPageScripts = (Component as any)?.unstable_scriptLoader

const pageIsDynamic = isDynamicRoute(pathname)

Expand Down Expand Up @@ -774,6 +775,14 @@ export async function renderToHTML(

let head: JSX.Element[] = defaultHead(inAmpMode)

let initialScripts: any = {}
if (hasPageScripts) {
initialScripts.beforeInteractive = []
.concat(hasPageScripts())
.filter((script: any) => script.props.strategy === 'beforeInteractive')
.map((script: any) => script.props)
}

let scriptLoader: any = {}
const nextExport =
!isSSG && (renderOpts.nextExport || (dev && (isAutoExport || isFallback)))
Expand Down Expand Up @@ -823,7 +832,7 @@ export async function renderToHTML(
updateScripts: (scripts) => {
scriptLoader = scripts
},
scripts: {},
scripts: initialScripts,
mountedInstances: new Set(),
}}
>
Expand Down
10 changes: 10 additions & 0 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -16,6 +16,7 @@ import {
isAssetError,
markAssetError,
} from '../../../client/route-loader'
import { handleClientScriptLoad } from '../../../client/script'
import isError, { getProperError } from '../../../lib/is-error'
import { denormalizePagePath } from '../../../server/denormalize-page-path'
import { normalizeLocalePath } from '../i18n/normalize-locale-path'
Expand Down Expand Up @@ -1275,6 +1276,15 @@ export default class Router implements BaseRouter {
)
let { error, props, __N_SSG, __N_SSP } = routeInfo

const component: any = routeInfo.Component
if (component && component.unstable_scriptLoader) {
const scripts = [].concat(component.unstable_scriptLoader())

scripts.forEach((script: any) => {
handleClientScriptLoad(script.props)
})
}

// handle redirect on client-transition
if ((__N_SSG || __N_SSP) && props) {
if (props.pageProps && props.pageProps.__N_REDIRECT) {
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/next-script-worker-strategy/index.test.ts
Expand Up @@ -12,7 +12,7 @@ describe('experimental.nextScriptWorkers: false with no Partytown dependency', (
files: {
'pages/index.js': `
import Script from 'next/script'

export default function Page() {
return (
<>
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('experimental.nextScriptWorkers: true with required Partytown dependenc
files: {
'pages/index.js': `
import Script from 'next/script'

export default function Page() {
return (
<>
Expand Down Expand Up @@ -123,7 +123,7 @@ describe('experimental.nextScriptWorkers: true with required Partytown dependenc
`document.querySelectorAll('script[type="text/partytown"]').length`
)

expect(predefinedWorkerScripts).toEqual(1)
expect(predefinedWorkerScripts).toBeGreaterThan(0)

await waitFor(1000)

Expand All @@ -132,7 +132,7 @@ describe('experimental.nextScriptWorkers: true with required Partytown dependenc
`document.querySelectorAll('script[type="text/partytown-x"]').length`
)

expect(processedWorkerScripts).toEqual(1)
expect(processedWorkerScripts).toBeGreaterThan(0)
} finally {
if (browser) await browser.close()
}
Expand Down Expand Up @@ -183,7 +183,7 @@ describe('experimental.nextScriptWorkers: true with config override', () => {
`,
'pages/index.js': `
import Script from 'next/script'

export default function Page() {
return (
<>
Expand Down
5 changes: 0 additions & 5 deletions test/integration/script-loader/base/pages/_app.js
Expand Up @@ -15,11 +15,6 @@ function MyApp({ Component, pageProps }) {
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentLazyOnload"
strategy="lazyOnload"
/>
<Script
id="documentBeforeInteractive"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentBeforeInteractive"
strategy="beforeInteractive"
/>
<Component {...pageProps} />
</>
)
Expand Down