Fix non-concurrent function _document (#31628)

This ensures functional `_document` is rendered correctly when not using concurrent mode. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes: #31593 x-ref: #30156
vercel · Nov 19, 2021 · e8ca334 · e8ca334
1 parent dab7b40
commit e8ca334
Show file tree

Hide file tree

Showing 11 changed files with 153 additions and 18 deletions.
diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx
@@ -1148,30 +1148,52 @@ export async function renderToHTML(
         styles: docProps.styles,
       }
     } else {
-      const bodyResult = async () => {
-        const content = (
-          <Body>
-            {ctx.err && ErrorDebug ? (
-              <ErrorDebug error={ctx.err} />
+      let bodyResult
+
+      if (concurrentFeatures) {
+        bodyResult = async () => {
+          // this must be called inside bodyResult so appWrappers is
+          // up to date when getWrappedApp is called
+          const content =
+            ctx.err && ErrorDebug ? (
+              <Body>
+                <ErrorDebug error={ctx.err} />
+              </Body>
             ) : (
-              getWrappedApp(
-                <App {...props} Component={Component} router={router} />
-              )
-            )}
-          </Body>
-        )
-
-        return concurrentFeatures
-          ? process.browser
+              <Body>
+                {getWrappedApp(
+                  <App {...props} Component={Component} router={router} />
+                )}
+              </Body>
+            )
+          return process.browser
             ? await renderToWebStream(content)
             : await renderToNodeStream(content, generateStaticHTML)
-          : piperFromArray([ReactDOMServer.renderToString(content)])
+        }
+      } else {
+        const content =
+          ctx.err && ErrorDebug ? (
+            <Body>
+              <ErrorDebug error={ctx.err} />
+            </Body>
+          ) : (
+            <Body>
+              {getWrappedApp(
+                <App {...props} Component={Component} router={router} />
+              )}
+            </Body>
+          )
+        // for non-concurrent rendering we need to ensure App is rendered
+        // before _document so that updateHead is called/collected before
+        // rendering _document's head
+        const result = piperFromArray([ReactDOMServer.renderToString(content)])
+        bodyResult = () => result
       }
 
       return {
         bodyResult,
         documentElement: () => (Document as any)(),
-        useMainContent: (fn?: (content: JSX.Element) => JSX.Element) => {
+        useMainContent: (fn?: (_content: JSX.Element) => JSX.Element) => {
           if (fn) {
             appWrappers.push(fn)
           }

diff --git a/test/e2e/next-head/app/components/meta.js b/test/e2e/next-head/app/components/meta.js
@@ -0,0 +1,12 @@
+import Head from 'next/head'
+
+export function Meta(props) {
+  return (
+    <>
+      <Head>
+        <meta name="test-head-3" content="hello" />
+        <meta name="test-head-4" content="hello" />
+      </Head>
+    </>
+  )
+}
diff --git a/test/e2e/next-head/app/pages/_document.js b/test/e2e/next-head/app/pages/_document.js
@@ -0,0 +1,13 @@
+import { Html, Head, Main, NextScript } from 'next/document'
+
+export default function MyDocument() {
+  return (
+    <Html>
+      <Head />
+      <body>
+        <Main />
+        <NextScript />
+      </body>
+    </Html>
+  )
+}
diff --git a/test/e2e/next-head/app/pages/index.js b/test/e2e/next-head/app/pages/index.js
@@ -0,0 +1,15 @@
+import Head from 'next/head'
+import { Meta } from '../components/meta'
+
+export default function Page(props) {
+  return (
+    <>
+      <Head>
+        <meta name="test-head-1" content="hello" />
+        <meta name="test-head-2" content="hello" />
+      </Head>
+      <Meta />
+      <p>index page</p>
+    </>
+  )
+}
diff --git a/test/e2e/next-head/index.test.ts b/test/e2e/next-head/index.test.ts
@@ -0,0 +1,41 @@
+import { createNext, FileRef } from 'e2e-utils'
+import { renderViaHTTP } from 'next-test-utils'
+import cheerio from 'cheerio'
+import webdriver from 'next-webdriver'
+import { NextInstance } from 'test/lib/next-modes/base'
+import { join } from 'path'
+
+describe('should set-up next', () => {
+  let next: NextInstance
+
+  beforeAll(async () => {
+    next = await createNext({
+      files: {
+        pages: new FileRef(join(__dirname, 'app/pages')),
+        components: new FileRef(join(__dirname, 'app/components')),
+      },
+    })
+  })
+  afterAll(() => next.destroy())
+
+  it('should have correct head tags in initial document', async () => {
+    const html = await renderViaHTTP(next.url, '/')
+    const $ = cheerio.load(html)
+
+    for (let i = 1; i < 5; i++) {
+      expect($(`meta[name="test-head-${i}"]`).attr()['content']).toBe('hello')
+    }
+  })
+
+  it('should have correct head tags after hydration', async () => {
+    const browser = await webdriver(next.url, '/')
+
+    for (let i = 1; i < 5; i++) {
+      expect(
+        await browser
+          .elementByCss(`meta[name="test-head-${i}"]`)
+          .getAttribute('content')
+      ).toBe('hello')
+    }
+  })
+})
diff --git a/...ent-functional-render-prop/lib/context.js → ...functional-render-prop/app/lib/context.js b/...ent-functional-render-prop/lib/context.js → ...functional-render-prop/app/lib/context.js
diff --git a/test/integration/document-functional-render-prop/app/next.config.js b/test/integration/document-functional-render-prop/app/next.config.js
@@ -0,0 +1,19 @@
+module.exports = {
+  experimental: {
+    reactRoot: true,
+    concurrentFeatures: true,
+  },
+  webpack(config) {
+    const { alias } = config.resolve
+    // FIXME: resolving react/jsx-runtime https://github.com/facebook/react/issues/20235
+    alias['react/jsx-dev-runtime'] = 'react/jsx-dev-runtime.js'
+    alias['react/jsx-runtime'] = 'react/jsx-runtime.js'
+
+    // Use react 18
+    alias['react'] = 'react-18'
+    alias['react-dom'] = 'react-dom-18'
+    alias['react-dom/server'] = 'react-dom-18/server'
+
+    return config
+  },
+}
diff --git a/test/integration/document-functional-render-prop/app/package.json b/test/integration/document-functional-render-prop/app/package.json
@@ -0,0 +1,12 @@
+{
+  "scripts": {
+    "next": "node -r ../test/require-hook.js ../../../../packages/next/dist/bin/next",
+    "dev": "yarn next dev",
+    "build": "yarn next build",
+    "start": "yarn next start"
+  },
+  "dependencies": {
+    "react": "*",
+    "react-dom": "*"
+  }
+}
diff --git a/...functional-render-prop/pages/_document.js → ...tional-render-prop/app/pages/_document.js b/...functional-render-prop/pages/_document.js → ...tional-render-prop/app/pages/_document.js
diff --git a/...ent-functional-render-prop/pages/index.js → ...functional-render-prop/app/pages/index.js b/...ent-functional-render-prop/pages/index.js → ...functional-render-prop/app/pages/index.js
diff --git a/test/integration/document-functional-render-prop/tests/index.test.js b/test/integration/document-functional-render-prop/tests/index.test.js
@@ -3,15 +3,16 @@
 import { join } from 'path'
 import { findPort, launchApp, killApp, renderViaHTTP } from 'next-test-utils'
 
-const appDir = join(__dirname, '..')
+const nodeArgs = ['-r', join(__dirname, '../../react-18/test/require-hook.js')]
+const appDir = join(__dirname, '../app')
 let appPort
 let app
 
 describe('Functional Custom Document', () => {
   describe('development mode', () => {
     beforeAll(async () => {
       appPort = await findPort()
-      app = await launchApp(appDir, appPort)
+      app = await launchApp(appDir, appPort, { nodeArgs })
     })
 
     afterAll(() => killApp(app))