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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: improve Pages Router server rendering performance #64461

Merged
merged 5 commits into from
Apr 16, 2024
Merged

perf: improve Pages Router server rendering performance #64461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7dee65d
add stuff
feedthejim Apr 14, 2024
baa6945
fix test
feedthejim Apr 14, 2024
a98c256
fix slight bug
feedthejim Apr 14, 2024
d3c9dfc
fix slight bug
feedthejim Apr 16, 2024
e376f18
Merge branch 'canary' into feedthejim/pages-render-optimisations
feedthejim Apr 16, 2024
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: 3 additions & 5 deletions packages/next/src/server/load-components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ async function loadComponentsImpl<N = any>({
let AppMod = {}
if (!isAppPath) {
;[DocumentMod, AppMod] = await Promise.all([
Promise.resolve().then(() => requirePage('/_document', distDir, false)),
Promise.resolve().then(() => requirePage('/_app', distDir, false)),
requirePage('/_document', distDir, false),
requirePage('/_app', distDir, false),
])
}

Expand Down Expand Up @@ -186,9 +186,7 @@ async function loadComponentsImpl<N = any>({
})
}

const ComponentMod = await Promise.resolve().then(() =>
requirePage(page, distDir, isAppPath)
)
const ComponentMod = await requirePage(page, distDir, isAppPath)

const Component = interopDefault(ComponentMod)
const Document = interopDefault(DocumentMod)
Expand Down
100 changes: 31 additions & 69 deletions packages/next/src/server/render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,8 @@ import { allowedStatusCodes, getRedirectStatus } from '../lib/redirect-status'
import RenderResult, { type PagesRenderResultMetadata } from './render-result'
import isError from '../lib/is-error'
import {
streamFromString,
streamToString,
chainStreams,
renderToInitialFizzStream,
continueFizzStream,
} from './stream-utils/node-web-streams-helper'
import { ImageConfigContext } from '../shared/lib/image-config-context.shared-runtime'
import stripAnsi from 'next/dist/compiled/strip-ansi'
Expand Down Expand Up @@ -471,11 +468,6 @@ export async function renderToHTMLImpl(
renderOpts.Component
const OriginComponent = Component

let serverComponentsInlinedTransformStream: TransformStream<
Uint8Array,
Uint8Array
> | null = null

const isFallback = !!query.__nextFallback
const notFoundSrcPage = query.__nextNotFoundSrcPage

Expand Down Expand Up @@ -1246,7 +1238,7 @@ export async function renderToHTMLImpl(
}

async function loadDocumentInitialProps(
renderShell?: (
renderShell: (
_App: AppType,
_Component: NextComponentType
) => Promise<ReactReadableStream>
Expand Down Expand Up @@ -1277,26 +1269,10 @@ export async function renderToHTMLImpl(
const { App: EnhancedApp, Component: EnhancedComponent } =
enhanceComponents(options, App, Component)

if (renderShell) {
return renderShell(EnhancedApp, EnhancedComponent).then(
async (stream) => {
await stream.allReady
const html = await streamToString(stream)
return { html, head }
}
)
}
const stream = await renderShell(EnhancedApp, EnhancedComponent)
await stream.allReady
const html = await streamToString(stream)

const html = await renderToString(
<Body>
<AppContainerWithIsomorphicFiberStructure>
{renderPageTree(EnhancedApp, EnhancedComponent, {
...props,
router,
})}
</AppContainerWithIsomorphicFiberStructure>
</Body>
)
return { html, head }
}
const documentCtx = { ...ctx, renderPage }
Expand Down Expand Up @@ -1349,48 +1325,39 @@ export async function renderToHTMLImpl(
})
}

const createBodyResult = getTracer().wrap(
RenderSpan.createBodyResult,
(initialStream: ReactReadableStream, suffix?: string) => {
return continueFizzStream(initialStream, {
suffix,
inlinedDataStream: serverComponentsInlinedTransformStream?.readable,
isStaticGeneration: true,
// this must be called inside bodyResult so appWrappers is
// up to date when `wrapApp` is called
getServerInsertedHTML: () => {
return renderToString(styledJsxInsertedHTML())
},
serverInsertedHTMLToHead: false,
validateRootLayout: undefined,
})
}
)

const hasDocumentGetInitialProps = !(
process.env.NEXT_RUNTIME === 'edge' || !Document.getInitialProps
)

let bodyResult: (s: string) => Promise<ReadableStream<Uint8Array>>
const hasDocumentGetInitialProps =
process.env.NEXT_RUNTIME !== 'edge' && !!Document.getInitialProps

// If it has getInitialProps, we will render the shell in `renderPage`.
// Otherwise we do it right now.
let documentInitialPropsRes:
| {}
| Awaited<ReturnType<typeof loadDocumentInitialProps>>
if (hasDocumentGetInitialProps) {
documentInitialPropsRes = await loadDocumentInitialProps(renderShell)
if (documentInitialPropsRes === null) return null
const { docProps } = documentInitialPropsRes as any
// includes suffix in initial html stream
bodyResult = (suffix: string) =>
createBodyResult(streamFromString(docProps.html + suffix))
} else {
const stream = await renderShell(App, Component)
bodyResult = (suffix: string) => createBodyResult(stream, suffix)
documentInitialPropsRes = {}

const [rawStyledJsxInsertedHTML, content] = await Promise.all([
renderToString(styledJsxInsertedHTML()),
(async () => {
if (hasDocumentGetInitialProps) {
documentInitialPropsRes = await loadDocumentInitialProps(renderShell)
if (documentInitialPropsRes === null) return null
const { docProps } = documentInitialPropsRes as any
return docProps.html
} else {
documentInitialPropsRes = {}
const stream = await renderShell(App, Component)
await stream.allReady
return streamToString(stream)
}
})(),
])

if (content === null) {
return null
}

const contentHTML = rawStyledJsxInsertedHTML + content

// @ts-ignore: documentInitialPropsRes is set
const { docProps } = (documentInitialPropsRes as any) || {}
const documentElement = (htmlProps: any) => {
if (process.env.NEXT_RUNTIME === 'edge') {
Expand All @@ -1410,7 +1377,7 @@ export async function renderToHTMLImpl(
}

return {
bodyResult,
contentHTML,
documentElement,
head,
headTags: [],
Expand Down Expand Up @@ -1577,12 +1544,7 @@ export async function renderToHTMLImpl(
prefix += '<!-- __NEXT_DATA__ -->'
}

const content = await streamToString(
chainStreams(
streamFromString(prefix),
await documentResult.bodyResult(renderTargetSuffix)
)
)
const content = prefix + documentResult.contentHTML + renderTargetSuffix
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense!


const optimizedHtml = await postProcessHTML(pathname, content, renderOpts, {
inAmpMode,
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/require.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ export function getPagePath(
return pagePath
}

export function requirePage(
export async function requirePage(
page: string,
distDir: string,
isAppPath: boolean
): any {
): Promise<any> {
const pagePath = getPagePath(page, distDir, undefined, isAppPath)
if (pagePath.endsWith('.html')) {
return promises.readFile(pagePath, 'utf8').catch((err) => {
Expand Down
1 change: 1 addition & 0 deletions packages/next/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ module.exports = ({ dev, turbo, bundleType, experimental }) => {
'this.serverOptions.experimentalTestProxy': JSON.stringify(false),
'this.minimalMode': JSON.stringify(true),
'this.renderOpts.dev': JSON.stringify(dev),
'renderOpts.dev': JSON.stringify(dev),
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit weird, similarly the this.serverOptions.experimentalTestProxy and this.minimalMode / this.renderOpts.dev are weird too, maybe these should be changed to process.env.NEXT_INTERNAL_X instead, not required for this PR though.

'process.env.NODE_ENV': JSON.stringify(
dev ? 'development' : 'production'
),
Expand Down
2 changes: 1 addition & 1 deletion test/unit/isolated/require-page.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('getPagePath', () => {

describe('requirePage', () => {
it('Should not find page /index when using /', async () => {
await expect(() => requirePage('/', distDir, false)).toThrow(
await expect(requirePage('/', distDir, false)).rejects.toThrow(
'Cannot find module for page: /'
)
})
Expand Down