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

Experimental script loader changes #22038

Merged
merged 53 commits into from
Mar 2, 2021
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
edabb95
Updating native-url version
janicklas-ralph Apr 4, 2020
c147099
Bump version
janicklas-ralph Apr 5, 2020
63b04ca
Merge branch 'canary' of github.com:zeit/next.js into canary
janicklas-ralph Apr 16, 2020
df52e5e
Merge branch 'canary' of github.com:zeit/next.js into canary
janicklas-ralph May 4, 2020
1baaf87
Merge branch 'canary' of github.com:zeit/next.js into canary
janicklas-ralph Aug 14, 2020
68bff2a
Merge branch 'canary' of github.com:zeit/next.js into canary
janicklas-ralph Aug 24, 2020
5f3c00a
Merge branch 'canary' of github.com:zeit/next.js into canary
janicklas-ralph Sep 24, 2020
38aa0af
ScriptLoader component
janicklas-ralph Sep 28, 2020
3dc9ec4
Adding testcases
janicklas-ralph Sep 28, 2020
0a5c068
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Sep 30, 2020
aab70cd
Fixing review comments
janicklas-ralph Oct 1, 2020
623e444
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Oct 1, 2020
f8a8a99
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Oct 6, 2020
6dbceb9
Fixing review comments
janicklas-ralph Oct 6, 2020
bf72597
Addressing review comments. Updating loading names and adding test ca…
janicklas-ralph Oct 15, 2020
612a45d
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Oct 15, 2020
8952a9d
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Oct 27, 2020
77cfe9c
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 6, 2020
2ed5656
Add unstable prefix to script loader component
janicklas-ralph Nov 6, 2020
cea970b
Execute onLoad each time the script is called
janicklas-ralph Nov 10, 2020
09093e1
Resolve review comments. Fire onload correctly
janicklas-ralph Nov 17, 2020
fb41fbb
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 17, 2020
6dc48ad
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 17, 2020
dab42f7
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 18, 2020
682083e
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 19, 2020
898b82f
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 20, 2020
35669c2
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 20, 2020
c6455c2
Resolving comments
janicklas-ralph Nov 20, 2020
59e3252
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 20, 2020
71d79c5
Fix lint
janicklas-ralph Nov 20, 2020
b820597
Replace key prop with id
janicklas-ralph Nov 20, 2020
18b98dc
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 20, 2020
215f527
Adding experimental.scriptLoader flag
janicklas-ralph Nov 20, 2020
6a39321
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 24, 2020
6e0a87e
Fix lint
janicklas-ralph Nov 24, 2020
125c600
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 25, 2020
81f0fe4
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Nov 30, 2020
26721fa
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Dec 1, 2020
a2b2234
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 8, 2021
6c75312
Making experimental script work in _document.js - Fixes for server to…
janicklas-ralph Feb 11, 2021
c4108c2
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 11, 2021
1e600e7
Remove console logs
janicklas-ralph Feb 11, 2021
2d85777
Fix ts errors
janicklas-ralph Feb 11, 2021
53ea044
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 11, 2021
2acfb44
Fix failing test
janicklas-ralph Feb 12, 2021
15a17b2
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 16, 2021
f11de5b
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 17, 2021
7c319d3
Resolve PR comment
janicklas-ralph Feb 17, 2021
0d6bf44
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 17, 2021
570ac9b
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 18, 2021
07b1a52
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 18, 2021
70826e4
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Feb 23, 2021
527cf2c
Merge branch 'canary' of github.com:zeit/next.js into script-loader
janicklas-ralph Mar 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,9 @@ export default async function getBaseWebpackConfig(
'process.env.__NEXT_OPTIMIZE_CSS': JSON.stringify(
config.experimental.optimizeCss && !dev
),
'process.env.__NEXT_SCRIPT_LOADER': JSON.stringify(
!!config.experimental.scriptLoader
),
'process.env.__NEXT_SCROLL_RESTORATION': JSON.stringify(
config.experimental.scrollRestoration
),
Expand Down
56 changes: 47 additions & 9 deletions packages/next/client/experimental-script.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { requestIdleCallback } from './request-idle-callback'
const ScriptCache = new Map()
const LoadCache = new Set()

interface Props extends ScriptHTMLAttributes<HTMLScriptElement> {
export interface Props extends ScriptHTMLAttributes<HTMLScriptElement> {
strategy?: 'defer' | 'lazy' | 'dangerouslyBlockRendering' | 'eager'
id?: string
onLoad?: () => void
Expand All @@ -16,13 +16,22 @@ interface Props extends ScriptHTMLAttributes<HTMLScriptElement> {
preload?: boolean
}

const ignoreProps = [
'onLoad',
'dangerouslySetInnerHTML',
'children',
'onError',
'strategy',
'preload',
]

const loadScript = (props: Props): void => {
const {
src = '',
src,
id,
onLoad = () => {},
dangerouslySetInnerHTML,
children = '',
id,
onError,
} = props

Expand Down Expand Up @@ -72,7 +81,7 @@ const loadScript = (props: Props): void => {
}

for (const [k, value] of Object.entries(props)) {
if (value === undefined) {
if (value === undefined || ignoreProps.includes(k)) {
continue
}

Expand All @@ -83,7 +92,32 @@ const loadScript = (props: Props): void => {
document.body.appendChild(el)
}

export default function Script(props: Props): JSX.Element | null {
function handleClientScriptLoad(props: Props) {
const { strategy = 'defer' } = props
if (strategy === 'defer') {
loadScript(props)
} else if (strategy === 'lazy') {
window.addEventListener('load', () => {
requestIdleCallback(() => loadScript(props))
})
}
}

function loadLazyScript(props: Props) {
if (document.readyState === 'complete') {
requestIdleCallback(() => loadScript(props))
} else {
window.addEventListener('load', () => {
Timer marked this conversation as resolved.
Show resolved Hide resolved
requestIdleCallback(() => loadScript(props))
})
}
}

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

function Script(props: Props): JSX.Element | null {
const {
src = '',
onLoad = () => {},
Expand All @@ -102,11 +136,13 @@ export default function Script(props: Props): JSX.Element | null {
if (strategy === 'defer') {
loadScript(props)
} else if (strategy === 'lazy') {
window.addEventListener('load', () => {
requestIdleCallback(() => loadScript(props))
})
loadLazyScript(props)
}
}, [strategy, props])
}, [props, strategy])

if (!process.env.__NEXT_SCRIPT_LOADER) {
return null
}
Comment on lines +143 to +145
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the entire import of this file is supposed to be blocked or stubbed out in the webpack configuration. Seems odd to have this here.

What happened to the next/experimental-script block in webpack-config.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tim removed here - #19937

Following the discussion, I think it was causing issues in webpack 5


if (strategy === 'dangerouslyBlockRendering') {
const syncProps: Props = { ...restProps }
Expand Down Expand Up @@ -157,3 +193,5 @@ export default function Script(props: Props): JSX.Element | null {

return null
}

export default Script
5 changes: 5 additions & 0 deletions packages/next/client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ if (process.env.__NEXT_I18N_SUPPORT) {
}
}

if (process.env.__NEXT_SCRIPT_LOADER && data.scriptLoader) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is being included in the main bundle despite being a conditional require
Any idea how to fix this @Timer @ijjk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see it being included in the main bundle in the stats here, was it previously being included?

const { initScriptLoader } = require('./experimental-script')
initScriptLoader(data.scriptLoader)
}

type RegisterFn = (input: [string, () => void]) => void

const pageLoader: PageLoader = new PageLoader(buildId, prefix)
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export type NEXT_DATA = {
locales?: string[]
defaultLocale?: string
domainLocales?: DomainLocales
scriptLoader?: any[]
isPreview?: boolean
}

Expand Down
40 changes: 38 additions & 2 deletions packages/next/pages/_document.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
} from '../next-server/server/get-page-files'
import { cleanAmpPath } from '../next-server/server/utils'
import { htmlEscapeJsonString } from '../server/htmlescape'
import Script, {
Props as ScriptLoaderProps,
} from '../client/experimental-script'

export { DocumentContext, DocumentInitialProps, DocumentProps }

Expand Down Expand Up @@ -313,6 +316,34 @@ export class Head extends Component<
]
}

handleDocumentScriptLoaderItems(children: React.ReactNode): ReactNode[] {
const { scriptLoader } = this.context
const scriptLoaderItems: ScriptLoaderProps[] = []
const filteredChildren: ReactNode[] = []

React.Children.forEach(children, (child: any) => {
if (child.type === Script) {
if (child.props.strategy === 'eager') {
scriptLoader.eager = (scriptLoader.eager || []).concat([
{
...child.props,
},
])
return
} else if (['lazy', 'defer'].includes(child.props.strategy)) {
scriptLoaderItems.push(child.props)
return
}
}

filteredChildren.push(child)
})

this.context.__NEXT_DATA__.scriptLoader = scriptLoaderItems

return filteredChildren
}

makeStylesheetInert(node: ReactNode): ReactNode[] {
return React.Children.map(node, (c: any) => {
if (
Expand Down Expand Up @@ -402,6 +433,10 @@ export class Head extends Component<
children = this.makeStylesheetInert(children)
}

if (process.env.__NEXT_SCRIPT_LOADER) {
children = this.handleDocumentScriptLoaderItems(children)
}

let hasAmphtmlRel = false
let hasCanonicalRel = false

Expand Down Expand Up @@ -649,10 +684,11 @@ export class NextScript extends Component<OriginProps> {
getPreNextScripts() {
const { scriptLoader } = this.context

return (scriptLoader.eager || []).map((file: string) => {
return (scriptLoader.eager || []).map((file: ScriptLoaderProps) => {
const { strategy, ...props } = file
return (
<script
{...file}
{...props}
nonce={this.props.nonce}
crossOrigin={
this.props.crossOrigin || process.env.__NEXT_CROSS_ORIGIN
Expand Down
21 changes: 21 additions & 0 deletions test/integration/script-loader/pages/_document.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react'
/// @ts-ignore
import Document, { Main, NextScript, Head } from 'next/document'
import Script from 'next/experimental-script'

export default class MyDocument extends Document {
constructor(props) {
Expand All @@ -19,6 +20,26 @@ export default class MyDocument extends Document {
rel="stylesheet"
href="https://fonts.googleapis.com/css?family=Voces"
/>
<Script
id="documentDefer"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentDefer"
strategy="defer"
></Script>
<Script
id="documentLazy"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentLazy"
strategy="lazy"
></Script>
<Script
id="documentBlock"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentBlock"
strategy="dangerouslyBlockRendering"
></Script>
<Script
id="documentEager"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=documentEager"
strategy="eager"
></Script>
</Head>
<body>
<Main />
Expand Down
4 changes: 2 additions & 2 deletions test/integration/script-loader/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const Page = () => {
return (
<div class="container">
<Script
id="script"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=defer"
id="scriptDefer"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptDefer"
preload
></Script>
<div>index</div>
Expand Down
4 changes: 2 additions & 2 deletions test/integration/script-loader/pages/page1.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const Page = () => {
return (
<div class="container">
<Script
id="script"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=eager"
id="scriptEager"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptEager"
strategy="eager"
></Script>
<div>page1</div>
Expand Down
2 changes: 1 addition & 1 deletion test/integration/script-loader/pages/page2.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const Page = () => {
return (
<div class="container">
<Script
id="script"
id="scriptBlock"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=dangerouslyBlockRendering"
strategy="dangerouslyBlockRendering"
></Script>
Expand Down
4 changes: 2 additions & 2 deletions test/integration/script-loader/pages/page3.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const Page = () => {
})`}
</Script>
<Script
id="script"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=lazy"
id="scriptLazy"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptLazy"
strategy="lazy"
></Script>
<div>page3</div>
Expand Down
Loading