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

Ensure there is only 1 render pass in concurrent rendering with getInitialProps in _document #36352

Merged
merged 3 commits into from Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 74 additions & 23 deletions packages/next/server/render.tsx
Expand Up @@ -395,7 +395,6 @@ const useFlightResponse = createFlightHook()

// Create the wrapper component for a Flight stream.
function createServerComponentRenderer(
AppMod: any,
ComponentMod: any,
{
cachePrefix,
Expand All @@ -415,10 +414,13 @@ function createServerComponentRenderer(
globalThis.__webpack_require__ = ComponentMod.__next_rsc__.__webpack_require__
const Component = interopDefault(ComponentMod)

function ServerComponentWrapper(props: any) {
function ServerComponentWrapper({ App, router, ...props }: any) {
const id = (React as any).useId()

const reqStream: ReadableStream<Uint8Array> = renderToReadableStream(
renderFlight(AppMod, ComponentMod, props),
<App>
<Component {...props} />
</App>,
serverComponentManifest
)

Expand Down Expand Up @@ -520,7 +522,7 @@ export async function renderToHTML(
if (isServerComponent) {
serverComponentsInlinedTransformStream = new TransformStream()
const search = urlQueryToSearchParams(query).toString()
Component = createServerComponentRenderer(AppMod, ComponentMod, {
Component = createServerComponentRenderer(ComponentMod, {
cachePrefix: pathname + (search ? `?${search}` : ''),
inlinedTransformStream: serverComponentsInlinedTransformStream,
staticTransformStream: serverComponentsPageDataTransformStream,
Expand Down Expand Up @@ -1311,11 +1313,24 @@ export async function renderToHTML(
}
}

async function documentInitialProps() {
async function documentInitialProps(
renderShell?: ({
EnhancedApp,
EnhancedComponent,
}: {
EnhancedApp?: AppType
EnhancedComponent?: NextComponentType
}) => Promise<void>
) {
const renderPage: RenderPage = (
options: ComponentsEnhancer = {}
): RenderPageResult | Promise<RenderPageResult> => {
if (ctx.err && ErrorDebug) {
// Always start rendering the shell even if there's an error.
if (renderShell) {
renderShell({})
}

const html = ReactDOMServer.renderToString(
<Body>
<ErrorDebug error={ctx.err} />
Expand All @@ -1333,6 +1348,14 @@ export async function renderToHTML(
const { App: EnhancedApp, Component: EnhancedComponent } =
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 html = ReactDOMServer.renderToString(
<Body>
<AppContainerWithIsomorphicFiberStructure>
Expand Down Expand Up @@ -1364,20 +1387,30 @@ export async function renderToHTML(
return { docProps, documentCtx }
}

const renderContent = () => {
const renderContent = ({
EnhancedApp,
EnhancedComponent,
}: {
EnhancedApp?: AppType
EnhancedComponent?: NextComponentType
} = {}) => {
return ctx.err && ErrorDebug ? (
<Body>
<ErrorDebug error={ctx.err} />
</Body>
) : (
<Body>
<AppContainerWithIsomorphicFiberStructure>
{isServerComponent && !!AppMod.__next_rsc__ ? (
// _app.server.js is used.
<Component {...props.pageProps} />
) : (
<App {...props} Component={Component} router={router} />
)}
{isServerComponent
? React.createElement(EnhancedComponent || Component, {
App: EnhancedApp || App,
shuding marked this conversation as resolved.
Show resolved Hide resolved
...props.pageProps,
})
: React.createElement(EnhancedApp || App, {
...props,
Component: EnhancedComponent || Component,
router,
})}
</AppContainerWithIsomorphicFiberStructure>
</Body>
)
Expand Down Expand Up @@ -1419,13 +1452,23 @@ export async function renderToHTML(
}
}
} else {
// We start rendering the shell earlier, before returning the head tags
// to `documentResult`.
const content = renderContent()
const renderStream = await renderToInitialStream({
ReactDOMServer,
element: content,
})
let renderStream: ReadableStream<Uint8Array> & {
allReady?: Promise<void> | undefined
}

const renderShell = async ({
EnhancedApp,
EnhancedComponent,
}: {
EnhancedApp?: AppType
EnhancedComponent?: NextComponentType
} = {}) => {
const content = renderContent({ EnhancedApp, EnhancedComponent })
renderStream = await renderToInitialStream({
ReactDOMServer,
element: content,
})
}

const bodyResult = async (suffix: string) => {
// this must be called inside bodyResult so appWrappers is
Expand Down Expand Up @@ -1494,10 +1537,18 @@ export async function renderToHTML(
!Document.getInitialProps
)

const documentInitialPropsRes = hasDocumentGetInitialProps
? await documentInitialProps()
: {}
if (documentInitialPropsRes === null) return null
// If it has getInitialProps, we will render the shell in `renderPage`.
// Otherwise we do it right now.
let documentInitialPropsRes:
| {}
| Awaited<ReturnType<typeof documentInitialProps>>
if (hasDocumentGetInitialProps) {
documentInitialPropsRes = await documentInitialProps(renderShell)
if (documentInitialPropsRes === null) return null
} else {
await renderShell()
documentInitialPropsRes = {}
}

const { docProps } = (documentInitialPropsRes as any) || {}
const documentElement = () => {
Expand Down
11 changes: 11 additions & 0 deletions test/development/basic/styled-components.test.ts
Expand Up @@ -73,4 +73,15 @@ describe('styled-components SWC transform', () => {
expect(html).toContain('background:transparent')
expect(html).toContain('color:white')
})

it('should only render once on the server per request', async () => {
const outputs = []
next.on('stdout', (args) => {
outputs.push(args)
})
await renderViaHTTP(next.url, '/')
expect(
outputs.filter((output) => output.trim() === '__render__').length
).toBe(1)
})
})
1 change: 1 addition & 0 deletions test/development/basic/styled-components/pages/index.js
Expand Up @@ -21,6 +21,7 @@ const Button = styled.a`
`

export default function Home() {
console.log('__render__')
return (
<div>
<Button
Expand Down
1 change: 1 addition & 0 deletions test/integration/react-18/app/pages/index.js
Expand Up @@ -5,6 +5,7 @@ export default function Index() {
if (typeof window !== 'undefined') {
window.didHydrate = true
}
console.log('__render__')
return (
<div>
<p id="react-dom-version">{ReactDOM.version}</p>
Expand Down
5 changes: 5 additions & 0 deletions test/integration/react-18/test/basics.js
Expand Up @@ -5,6 +5,11 @@ import cheerio from 'cheerio'
import { renderViaHTTP } from 'next-test-utils'

export default (context, env) => {
it('should only render once in SSR', async () => {
await renderViaHTTP(context.appPort, '/')
expect([...context.stdout.matchAll(/__render__/g)].length).toBe(1)
})

it('no warnings for image related link props', async () => {
await renderViaHTTP(context.appPort, '/')
expect(context.stderr).not.toContain('Warning: Invalid DOM property')
Expand Down
6 changes: 6 additions & 0 deletions test/lib/next-test-utils.js
Expand Up @@ -758,6 +758,10 @@ function runSuite(suiteName, context, options) {
const onStderr = (msg) => {
context.stderr += msg
}
context.stdout = ''
const onStdout = (msg) => {
context.stdout += msg
}
if (env === 'prod') {
context.appPort = await findPort()
const { stdout, stderr, code } = await nextBuild(appDir, [], {
Expand All @@ -769,11 +773,13 @@ function runSuite(suiteName, context, options) {
context.code = code
context.server = await nextStart(context.appDir, context.appPort, {
onStderr,
onStdout,
})
} else if (env === 'dev') {
context.appPort = await findPort()
context.server = await launchApp(context.appDir, context.appPort, {
onStderr,
onStdout,
})
}
})
Expand Down