Skip to content

Commit

Permalink
Currently all scripts that are required for every page are loaded as …
Browse files Browse the repository at this point in the history
…part of the bootstrap scripts API in React. Unfortunately this loads them all as sync scripts and thus requires preloading which increases their priority higher than they might otherwise be causing things like images to load later than desired, blocking paint. We can improve this by only using one script for bootstrapping and having the rest pre-initialized. This only works because all of these scripts are webpack runtime or chunks and can be loaded in any order asynchronously.

With this change we should see improvements in LCP and other metrics as preloads for images are favored over loading scripts
  • Loading branch information
gnoff committed Aug 8, 2023
1 parent ee15b3b commit 3f5ad71
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 38 deletions.
66 changes: 28 additions & 38 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import { appendMutableCookies } from '../web/spec-extension/adapters/request-coo
import { ComponentsType } from '../../build/webpack/loaders/next-app-loader'
import { ModuleReference } from '../../build/webpack/loaders/metadata/types'
import { createServerInsertedHTML } from './server-inserted-html'
import { getRequiredScripts } from './required-scripts'

export type GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
Expand Down Expand Up @@ -1387,11 +1388,15 @@ export async function renderToHTMLOrFlight(
* A new React Component that renders the provided React Component
* using Flight which can then be rendered to HTML.
*/
const createServerComponentsRenderer = (loaderTreeToRender: LoaderTree) =>
const createServerComponentsRenderer = (
loaderTreeToRender: LoaderTree,
preinitScripts: () => void
) =>
createServerComponentRenderer<{
asNotFound: boolean
}>(
async (props) => {
preinitScripts()
// Create full component tree from root to leaf.
const injectedCSS = new Set<string>()
const injectedFontPreloadTags = new Set<string>()
Expand Down Expand Up @@ -1490,7 +1495,16 @@ export async function renderToHTMLOrFlight(
integrity: subresourceIntegrityManifest?.[polyfill],
}))

const ServerComponentsRenderer = createServerComponentsRenderer(tree)
const [preinitScripts, bootstrapScript] = getRequiredScripts(
buildManifest,
assetPrefix,
subresourceIntegrityManifest,
getAssetQueryString(true)
)
const ServerComponentsRenderer = createServerComponentsRenderer(
tree,
preinitScripts
)
const content = (
<HeadManagerContext.Provider
value={{
Expand Down Expand Up @@ -1576,28 +1590,7 @@ export async function renderToHTMLOrFlight(
onError: htmlRendererErrorHandler,
nonce,
// Include hydration scripts in the HTML
bootstrapScripts: [
...(subresourceIntegrityManifest
? buildManifest.rootMainFiles.map((src) => ({
src:
`${assetPrefix}/_next/` +
src +
// Always include the timestamp query in development
// as Safari caches them during the same session, no
// matter what cache headers are set.
getAssetQueryString(true),
integrity: subresourceIntegrityManifest[src],
}))
: buildManifest.rootMainFiles.map(
(src) =>
`${assetPrefix}/_next/` +
src +
// Always include the timestamp query in development
// as Safari caches them during the same session, no
// matter what cache headers are set.
getAssetQueryString(true)
)),
],
bootstrapScripts: [bootstrapScript],
},
})

Expand Down Expand Up @@ -1680,8 +1673,18 @@ export async function renderToHTMLOrFlight(
)}
</>
)

const [errorPreinitScripts, errorBootstrapScript] =
getRequiredScripts(
buildManifest,
assetPrefix,
subresourceIntegrityManifest,
getAssetQueryString(false)
)

const ErrorPage = createServerComponentRenderer(
async () => {
errorPreinitScripts()
const [MetadataTree, MetadataOutlet] = createMetadataComponents({
tree, // still use original tree with not-found boundaries to extract metadata
pathname,
Expand Down Expand Up @@ -1762,20 +1765,7 @@ export async function renderToHTMLOrFlight(
streamOptions: {
nonce,
// Include hydration scripts in the HTML
bootstrapScripts: subresourceIntegrityManifest
? buildManifest.rootMainFiles.map((src) => ({
src:
`${assetPrefix}/_next/` +
src +
getAssetQueryString(false),
integrity: subresourceIntegrityManifest[src],
}))
: buildManifest.rootMainFiles.map(
(src) =>
`${assetPrefix}/_next/` +
src +
getAssetQueryString(false)
),
bootstrapScripts: [errorBootstrapScript],
},
})

Expand Down
56 changes: 56 additions & 0 deletions packages/next/src/server/app-render/required-scripts.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import type { BuildManifest } from '../get-page-files'

import ReactDOM from 'react-dom'

export function getRequiredScripts(
buildManifest: BuildManifest,
assetPrefix: string,
SRIManifest: undefined | Record<string, string>,
qs: string
): [() => void, string | { src: string; integrity: string }] {
let preinitScripts: () => void
let preinitScriptCommands: string[] = []
let bootstrapScript: string | { src: string; integrity: string } = ''
const files = buildManifest.rootMainFiles
if (files.length === 0) {
throw new Error(
'Invariant: missing bootstrap script. This is a bug in Next.js'
)
}
if (SRIManifest) {
bootstrapScript = {
src: `${assetPrefix}/_next/` + files[0] + qs,
integrity: SRIManifest[files[0]],
}
for (let i = 1; i < files.length; i++) {
const src = `${assetPrefix}/_next/` + files[i] + qs
const integrity = SRIManifest[files[i]]
preinitScriptCommands.push(src, integrity)
}
preinitScripts = () => {
// preinitScriptCommands is a double indexed array of src/integrity pairs
for (let i = 0; i < preinitScriptCommands.length; i += 2) {
ReactDOM.preinit(preinitScriptCommands[i], {
as: 'script',
integrity: preinitScriptCommands[i + 1],
})
}
}
} else {
bootstrapScript = `${assetPrefix}/_next/` + files[0] + qs
for (let i = 1; i < files.length; i++) {
const src = `${assetPrefix}/_next/` + files[i] + qs
preinitScriptCommands.push(src)
}
preinitScripts = () => {
// preinitScriptCommands is a singled indexed array of src values
for (let i = 0; i < preinitScriptCommands.length; i++) {
ReactDOM.preinit(preinitScriptCommands[i], {
as: 'script',
})
}
}
}

return [preinitScripts, bootstrapScript]
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app/app/bootstrap/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async function Page() {
return (
<div id="anchor">hello, bootstrap scripts should appear after this div</div>
)
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/app/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1859,5 +1859,16 @@ createNextDescribe(
expect(await browser.elementByCss('p').text()).toBe('item count 128000')
})
})

describe('bootstrap scripts', () => {
it('should only bootstrap with one script, prinitializing the rest', async () => {
const html = await next.render('/bootstrap')
const $ = cheerio.load(html)

// We assume a minimum of 2 scripts, webpack runtime + main-app
expect($('script[async]').length).toBeGreaterThan(1)
expect($('body').find('script[async]').length).toBe(1)
})
})
}
)

0 comments on commit 3f5ad71

Please sign in to comment.