From 49b652d303b01b718724a429b418bfafc724fccc Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 02:12:05 +0200 Subject: [PATCH 1/6] Wait for shell resolve with gIP is set --- packages/next/server/render.tsx | 35 +++++++++++-------- .../amphtml-fragment-style/next.config.js | 2 -- .../amphtml-fragment-style/test/index.test.js | 9 ++--- .../styled-jsx-module/app/next.config.js | 4 --- .../styled-jsx-module/test/index.test.js | 13 ++----- 5 files changed, 26 insertions(+), 37 deletions(-) delete mode 100644 test/integration/amphtml-fragment-style/next.config.js delete mode 100644 test/integration/styled-jsx-module/app/next.config.js diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index ca0b4c5111b3..94236ceb2b2c 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -263,6 +263,10 @@ export type RenderOptsPartial = { export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial +type ReactReadableStream = ReadableStream & { + allReady?: Promise | undefined +} + const invalidKeysMsg = ( methodName: 'getServerSideProps' | 'getStaticProps', invalidKeys: string[] @@ -1344,11 +1348,11 @@ export async function renderToHTML( renderShell?: ( _App: AppType, _Component: NextComponentType - ) => Promise + ) => Promise ) { - const renderPage: RenderPage = ( + const renderPage: RenderPage = async ( options: ComponentsEnhancer = {} - ): RenderPageResult | Promise => { + ): Promise => { if (ctx.err && ErrorDebug) { // Always start rendering the shell even if there's an error. if (renderShell) { @@ -1373,11 +1377,10 @@ export async function renderToHTML( enhanceComponents(options, App, Component) if (renderShell) { - return renderShell(EnhancedApp, EnhancedComponent).then(() => { - // When using concurrent features, we don't have or need the full - // html so it's fine to return nothing here. - return { html: '', head } - }) + const stream = await renderShell(EnhancedApp, EnhancedComponent) + await stream.allReady + const html = await streamToString(stream) + return { html, head } } const html = ReactDOMServer.renderToString( @@ -1473,9 +1476,7 @@ export async function renderToHTML( } } else { // Enabling react concurrent rendering mode: __NEXT_REACT_ROOT = true - let renderStream: ReadableStream & { - allReady?: Promise | undefined - } + let renderStream: ReactReadableStream const renderShell = async ( EnhancedApp: AppType, @@ -1486,12 +1487,16 @@ export async function renderToHTML( ReactDOMServer, element: content, }) + return renderStream } - const bodyResult = async (suffix: string) => { + let bodyResult: ( + s: string + ) => Promise> | ReadableStream + + bodyResult = (suffix) => { // this must be called inside bodyResult so appWrappers is // up to date when `wrapApp` is called - const flushEffectHandler = (): string => { const allFlushEffects = [ styledJsxFlushEffect, @@ -1539,7 +1544,7 @@ export async function renderToHTML( // be streamed. // Do not use `await` here. generateStaticFlightDataIfNeeded() - return await continueFromInitialStream({ + return continueFromInitialStream({ renderStream, suffix, dataStream: serverComponentsInlinedTransformStream?.readable, @@ -1562,6 +1567,8 @@ export async function renderToHTML( if (hasDocumentGetInitialProps) { documentInitialPropsRes = await documentInitialProps(renderShell) if (documentInitialPropsRes === null) return null + const { docProps } = documentInitialPropsRes as any + bodyResult = (suffix) => streamFromArray([docProps.html, suffix]) } else { await renderShell(App, Component) documentInitialPropsRes = {} diff --git a/test/integration/amphtml-fragment-style/next.config.js b/test/integration/amphtml-fragment-style/next.config.js deleted file mode 100644 index 9c1c065bdfe3..000000000000 --- a/test/integration/amphtml-fragment-style/next.config.js +++ /dev/null @@ -1,2 +0,0 @@ -const path = require('path') -module.exports = require(path.join(__dirname, '../../lib/with-react-17.js'))({}) diff --git a/test/integration/amphtml-fragment-style/test/index.test.js b/test/integration/amphtml-fragment-style/test/index.test.js index 3bccdd030339..e7e83e705316 100644 --- a/test/integration/amphtml-fragment-style/test/index.test.js +++ b/test/integration/amphtml-fragment-style/test/index.test.js @@ -12,19 +12,14 @@ import { } from 'next-test-utils' const appDir = join(__dirname, '../') -const nodeArgs = ['-r', join(appDir, '../../lib/react-17-require-hook.js')] let appPort let app describe('AMP Fragment Styles', () => { beforeAll(async () => { - await nextBuild(appDir, [], { - nodeArgs, - }) + await nextBuild(appDir, []) appPort = await findPort() - app = await nextStart(appDir, appPort, { - nodeArgs, - }) + app = await nextStart(appDir, appPort) }) afterAll(() => killApp(app)) diff --git a/test/integration/styled-jsx-module/app/next.config.js b/test/integration/styled-jsx-module/app/next.config.js deleted file mode 100644 index 40b089d571fe..000000000000 --- a/test/integration/styled-jsx-module/app/next.config.js +++ /dev/null @@ -1,4 +0,0 @@ -const path = require('path') -module.exports = require(path.join(__dirname, '../../../lib/with-react-17.js'))( - {} -) diff --git a/test/integration/styled-jsx-module/test/index.test.js b/test/integration/styled-jsx-module/test/index.test.js index 0243f07266eb..3bad07cb13c4 100644 --- a/test/integration/styled-jsx-module/test/index.test.js +++ b/test/integration/styled-jsx-module/test/index.test.js @@ -12,7 +12,6 @@ import { } from 'next-test-utils' const appDir = join(__dirname, '../app') -const nodeArgs = ['-r', join(appDir, '../../../lib/react-17-require-hook.js')] let appPort let app @@ -49,13 +48,9 @@ function runTests() { describe('styled-jsx using in node_modules', () => { describe('Production', () => { beforeAll(async () => { - await nextBuild(appDir, undefined, { - nodeArgs, - }) + await nextBuild(appDir) appPort = await findPort() - app = await nextStart(appDir, appPort, { - nodeArgs, - }) + app = await nextStart(appDir, appPort) }) afterAll(() => killApp(app)) @@ -65,9 +60,7 @@ describe('styled-jsx using in node_modules', () => { describe('Development', () => { beforeAll(async () => { appPort = await findPort() - app = await launchApp(appDir, appPort, { - nodeArgs, - }) + app = await launchApp(appDir, appPort) }) afterAll(() => killApp(app)) From 4f765590cee59b4aab855377d9c80effff7e4599 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 02:39:49 +0200 Subject: [PATCH 2/6] requirePage should not always be async --- packages/next/server/render.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 94236ceb2b2c..e5f3b2265374 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -1350,9 +1350,9 @@ export async function renderToHTML( _Component: NextComponentType ) => Promise ) { - const renderPage: RenderPage = async ( + const renderPage: RenderPage = ( options: ComponentsEnhancer = {} - ): Promise => { + ): RenderPageResult | Promise => { if (ctx.err && ErrorDebug) { // Always start rendering the shell even if there's an error. if (renderShell) { @@ -1377,10 +1377,13 @@ export async function renderToHTML( enhanceComponents(options, App, Component) if (renderShell) { - const stream = await renderShell(EnhancedApp, EnhancedComponent) - await stream.allReady - const html = await streamToString(stream) - return { html, head } + return renderShell(EnhancedApp, EnhancedComponent).then( + async (stream) => { + await stream.allReady + const html = await streamToString(stream) + return { html, head } + } + ) } const html = ReactDOMServer.renderToString( From ea146656e69babe0ffebf4a27393579d49d6c441 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 03:20:07 +0200 Subject: [PATCH 3/6] fix flush effects --- .../next/server/node-web-streams-helper.ts | 40 +++--- packages/next/server/render.tsx | 123 +++++++++--------- packages/next/server/view-render.tsx | 3 +- 3 files changed, 81 insertions(+), 85 deletions(-) diff --git a/packages/next/server/node-web-streams-helper.ts b/packages/next/server/node-web-streams-helper.ts index 6dc7e46ce0e3..353ccb3f70d9 100644 --- a/packages/next/server/node-web-streams-helper.ts +++ b/packages/next/server/node-web-streams-helper.ts @@ -1,5 +1,9 @@ import { nonNullable } from '../lib/non-nullable' +export type ReactReadableStream = ReadableStream & { + allReady?: Promise | undefined +} + export function readableStreamTee( readable: ReadableStream ): [ReadableStream, ReadableStream] { @@ -138,29 +142,24 @@ export function renderToInitialStream({ }: { ReactDOMServer: any element: React.ReactElement -}): Promise< - ReadableStream & { - allReady?: Promise - } -> { +}): Promise { return ReactDOMServer.renderToReadableStream(element) } -export async function continueFromInitialStream({ - suffix, - dataStream, - generateStaticHTML, - flushEffectHandler, - renderStream, -}: { - suffix?: string - dataStream?: ReadableStream - generateStaticHTML: boolean - flushEffectHandler?: () => string - renderStream: ReadableStream & { - allReady?: Promise +export async function continueFromInitialStream( + renderStream: ReactReadableStream, + { + suffix, + dataStream, + generateStaticHTML, + flushEffectHandler, + }: { + suffix?: string + dataStream?: ReadableStream + generateStaticHTML: boolean + flushEffectHandler?: () => string } -}): Promise> { +): Promise> { const closeTag = '' const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null @@ -198,12 +197,11 @@ export async function renderToStream({ flushEffectHandler?: () => string }): Promise> { const renderStream = await renderToInitialStream({ ReactDOMServer, element }) - return continueFromInitialStream({ + return continueFromInitialStream(renderStream, { suffix, dataStream, generateStaticHTML, flushEffectHandler, - renderStream, }) } diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index e5f3b2265374..e4a203322381 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -20,6 +20,7 @@ import type { FontManifest } from './font-utils' import type { LoadComponentsReturnType, ManifestItem } from './load-components' import type { GetServerSideProps, GetStaticProps, PreviewData } from '../types' import type { UnwrapPromise } from '../lib/coalesced-function' +import type { ReactReadableStream } from './node-web-streams-helper' import React from 'react' import { createFromReadableStream } from 'next/dist/compiled/react-server-dom-webpack' @@ -263,10 +264,6 @@ export type RenderOptsPartial = { export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial -type ReactReadableStream = ReadableStream & { - allReady?: Promise | undefined -} - const invalidKeysMsg = ( methodName: 'getServerSideProps' | 'getStaticProps', invalidKeys: string[] @@ -1493,68 +1490,65 @@ export async function renderToHTML( return renderStream } - let bodyResult: ( - s: string - ) => Promise> | ReadableStream - - bodyResult = (suffix) => { - // this must be called inside bodyResult so appWrappers is - // up to date when `wrapApp` is called - const flushEffectHandler = (): string => { - const allFlushEffects = [ - styledJsxFlushEffect, - ...(flushEffects || []), - ] - const flushed = ReactDOMServer.renderToString( - <> - {allFlushEffects.map((flushEffect, i) => ( - {flushEffect()} - ))} - - ) - return flushed - } + const createBodyResult = + (initialStream: ReactReadableStream): typeof bodyResult => + (suffix) => { + // this must be called inside bodyResult so appWrappers is + // up to date when `wrapApp` is called + const flushEffectHandler = (): string => { + const allFlushEffects = [ + styledJsxFlushEffect, + ...(flushEffects || []), + ] + const flushed = ReactDOMServer.renderToString( + <> + {allFlushEffects.map((flushEffect, i) => ( + {flushEffect()} + ))} + + ) + return flushed + } - // Handle static data for server components. - async function generateStaticFlightDataIfNeeded() { - if (serverComponentsPageDataTransformStream) { - // If it's a server component with the Node.js runtime, we also - // statically generate the page data. - let data = '' - - const readable = serverComponentsPageDataTransformStream.readable - const reader = readable.getReader() - const textDecoder = new TextDecoder() - - while (true) { - const { done, value } = await reader.read() - if (done) { - break + // Handle static data for server components. + async function generateStaticFlightDataIfNeeded() { + if (serverComponentsPageDataTransformStream) { + // If it's a server component with the Node.js runtime, we also + // statically generate the page data. + let data = '' + + const readable = serverComponentsPageDataTransformStream.readable + const reader = readable.getReader() + const textDecoder = new TextDecoder() + + while (true) { + const { done, value } = await reader.read() + if (done) { + break + } + data += decodeText(value, textDecoder) } - data += decodeText(value, textDecoder) - } - ;(renderOpts as any).pageData = { - ...(renderOpts as any).pageData, - __flight__: data, + ;(renderOpts as any).pageData = { + ...(renderOpts as any).pageData, + __flight__: data, + } + return data } - return data } - } - // @TODO: A potential improvement would be to reuse the inlined - // data stream, or pass a callback inside as this doesn't need to - // be streamed. - // Do not use `await` here. - generateStaticFlightDataIfNeeded() - return continueFromInitialStream({ - renderStream, - suffix, - dataStream: serverComponentsInlinedTransformStream?.readable, - generateStaticHTML, - flushEffectHandler, - }) - } + // @TODO: A potential improvement would be to reuse the inlined + // data stream, or pass a callback inside as this doesn't need to + // be streamed. + // Do not use `await` here. + generateStaticFlightDataIfNeeded() + return continueFromInitialStream(initialStream, { + suffix, + dataStream: serverComponentsInlinedTransformStream?.readable, + generateStaticHTML, + flushEffectHandler, + }) + } const hasDocumentGetInitialProps = !( isServerComponent || @@ -1562,6 +1556,10 @@ export async function renderToHTML( !Document.getInitialProps ) + let bodyResult: ( + s: string + ) => Promise> | ReadableStream + // If it has getInitialProps, we will render the shell in `renderPage`. // Otherwise we do it right now. let documentInitialPropsRes: @@ -1571,9 +1569,10 @@ export async function renderToHTML( documentInitialPropsRes = await documentInitialProps(renderShell) if (documentInitialPropsRes === null) return null const { docProps } = documentInitialPropsRes as any - bodyResult = (suffix) => streamFromArray([docProps.html, suffix]) + bodyResult = createBodyResult(streamFromArray([docProps.html])) } else { - await renderShell(App, Component) + const stream = await renderShell(App, Component) + bodyResult = createBodyResult(stream) documentInitialPropsRes = {} } diff --git a/packages/next/server/view-render.tsx b/packages/next/server/view-render.tsx index f304f2472ce6..c486b7d77612 100644 --- a/packages/next/server/view-render.tsx +++ b/packages/next/server/view-render.tsx @@ -500,8 +500,7 @@ export async function renderToHTML( // Do not use `await` here. // generateStaticFlightDataIfNeeded() - return await continueFromInitialStream({ - renderStream, + return await continueFromInitialStream(renderStream, { suffix: '', dataStream: serverComponentsInlinedTransformStream?.readable, generateStaticHTML: generateStaticHTML || !hasConcurrentFeatures, From e06a67c1c4eb7d44e1628d11c5ecd1c623c0d2bf Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 07:32:02 +0200 Subject: [PATCH 4/6] tweak naming and typing --- packages/next/server/render.tsx | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index e4a203322381..50d650245c79 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -1341,7 +1341,7 @@ export async function renderToHTML( } } - async function documentInitialProps( + async function loadDocumentInitialProps( renderShell?: ( _App: AppType, _Component: NextComponentType @@ -1441,9 +1441,9 @@ export async function renderToHTML( if (!process.env.__NEXT_REACT_ROOT) { // Enabling react legacy rendering mode: __NEXT_REACT_ROOT = false if (Document.getInitialProps) { - const documentInitialPropsRes = await documentInitialProps() - if (documentInitialPropsRes === null) return null - const { docProps, documentCtx } = documentInitialPropsRes + const documentInitialProps = await loadDocumentInitialProps() + if (documentInitialProps === null) return null + const { docProps, documentCtx } = documentInitialProps return { bodyResult: (suffix: string) => @@ -1476,23 +1476,19 @@ export async function renderToHTML( } } else { // Enabling react concurrent rendering mode: __NEXT_REACT_ROOT = true - let renderStream: ReactReadableStream - const renderShell = async ( EnhancedApp: AppType, EnhancedComponent: NextComponentType ) => { const content = renderContent(EnhancedApp, EnhancedComponent) - renderStream = await renderToInitialStream({ + return await renderToInitialStream({ ReactDOMServer, element: content, }) - return renderStream } const createBodyResult = - (initialStream: ReactReadableStream): typeof bodyResult => - (suffix) => { + (initialStream: ReactReadableStream) => (suffix: string) => { // this must be called inside bodyResult so appWrappers is // up to date when `wrapApp` is called const flushEffectHandler = (): string => { @@ -1556,17 +1552,15 @@ export async function renderToHTML( !Document.getInitialProps ) - let bodyResult: ( - s: string - ) => Promise> | ReadableStream + let bodyResult: (s: string) => Promise> // If it has getInitialProps, we will render the shell in `renderPage`. // Otherwise we do it right now. let documentInitialPropsRes: | {} - | Awaited> + | Awaited> if (hasDocumentGetInitialProps) { - documentInitialPropsRes = await documentInitialProps(renderShell) + documentInitialPropsRes = await loadDocumentInitialProps(renderShell) if (documentInitialPropsRes === null) return null const { docProps } = documentInitialPropsRes as any bodyResult = createBodyResult(streamFromArray([docProps.html])) From f07b58b7237abd1c0b4964705646ccda4b3b91d2 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 14:04:20 +0200 Subject: [PATCH 5/6] tee shell stream --- packages/next/server/render.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 50d650245c79..fb2e9f9e3639 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -1376,8 +1376,8 @@ export async function renderToHTML( if (renderShell) { return renderShell(EnhancedApp, EnhancedComponent).then( async (stream) => { - await stream.allReady - const html = await streamToString(stream) + const forwardStream = readableStreamTee(stream)[1] + const html = await streamToString(forwardStream) return { html, head } } ) From 7ad8085851f023125aca99c09c134b6170389a2d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 May 2022 14:29:22 +0200 Subject: [PATCH 6/6] add test --- test/integration/app-document/pages/_document.js | 2 ++ test/integration/app-document/pages/index.js | 2 ++ test/integration/app-document/test/rendering.js | 8 ++++++++ 3 files changed, 12 insertions(+) diff --git a/test/integration/app-document/pages/_document.js b/test/integration/app-document/pages/_document.js index e78e002ffcea..c6a9c36e4143 100644 --- a/test/integration/app-document/pages/_document.js +++ b/test/integration/app-document/pages/_document.js @@ -42,6 +42,7 @@ export default class MyDocument extends Document { return { ...result, + cssInJsCount: (result.html.match(/css-in-js-class/g) || []).length, customProperty: 'Hello Document', withCSP: ctx.query.withCSP, } @@ -74,6 +75,7 @@ export default class MyDocument extends Document {

Hello Document HMR

+
{this.props.cssInJsCount}
) diff --git a/test/integration/app-document/pages/index.js b/test/integration/app-document/pages/index.js index c4ab35053abe..f21ecbe1dd7a 100644 --- a/test/integration/app-document/pages/index.js +++ b/test/integration/app-document/pages/index.js @@ -2,8 +2,10 @@ import Link from 'next/link' export default () => (
index
+ about +
) diff --git a/test/integration/app-document/test/rendering.js b/test/integration/app-document/test/rendering.js index 541594eef04d..71514580fef7 100644 --- a/test/integration/app-document/test/rendering.js +++ b/test/integration/app-document/test/rendering.js @@ -22,6 +22,14 @@ export default function ({ app }, suiteName, render, fetch) { expect($('body').hasClass('custom_class')).toBe(true) }) + it('Document.getInitialProps returns html prop representing app shell', async () => { + // Extract css-in-js-class from the rendered HTML, which is returned by Document.getInitialProps + const $index = await get$('/') + const $about = await get$('/about') + expect($index('#css-in-cjs-count').text()).toBe('2') + expect($about('#css-in-cjs-count').text()).toBe('0') + }) + test('It injects custom head tags', async () => { const $ = await get$('/') expect($('head').text()).toMatch('body { margin: 0 }')