Fix react root env missing in NextServer (#37562)

* Fix react root env missing in NextServer * switch to useId instead of using process.env var * add production test * extend timeout * fix test * fix lint * use version to detect if enable react root
vercel · Jun 9, 2022 · 0bfdf0e · 0bfdf0e
1 parent 9155201
commit 0bfdf0e
Show file tree

Hide file tree

Showing 12 changed files with 179 additions and 63 deletions.
diff --git a/packages/next/bin/next.ts b/packages/next/bin/next.ts
@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 import * as log from '../build/output/log'
 import arg from 'next/dist/compiled/arg/index.js'
+import React from 'react'
 import { NON_STANDARD_NODE_ENV } from '../lib/constants'
 ;['react', 'react-dom'].forEach((dependency) => {
   try {
@@ -42,9 +43,6 @@ const args = arg(
   }
 )
 
-// Detect if react-dom is enabled streaming rendering mode
-const shouldUseReactRoot = !!require('react-dom/server').renderToPipeableStream
-
 // Version is inlined into the file using taskr build pipeline
 if (args['--version']) {
   console.log(`Next.js v${process.env.__NEXT_VERSION}`)
@@ -108,6 +106,8 @@ if (process.env.NODE_ENV) {
 
 ;(process.env as any).NODE_ENV = process.env.NODE_ENV || defaultEnv
 ;(process.env as any).NEXT_RUNTIME = 'nodejs'
+
+const shouldUseReactRoot = parseInt(React.version) >= 18
 if (shouldUseReactRoot) {
   ;(process.env as any).__NEXT_REACT_ROOT = 'true'
 }

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
@@ -25,6 +25,7 @@ import type {
 import fs from 'fs'
 import { join, relative, resolve, sep } from 'path'
 import { IncomingMessage, ServerResponse } from 'http'
+import React from 'react'
 import { addRequestMeta, getRequestMeta } from './request-meta'
 
 import {
@@ -82,6 +83,11 @@ import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-
 import { clonableBodyForRequest } from './body-streams'
 import { getNextPathnameInfo } from '../shared/lib/router/utils/get-next-pathname-info'
 
+const shouldUseReactRoot = parseInt(React.version) >= 18
+if (shouldUseReactRoot) {
+  ;(process.env as any).__NEXT_REACT_ROOT = 'true'
+}
+
 export * from './base-server'
 
 type ExpressMiddleware = (

diff --git a/packages/next/server/next.ts b/packages/next/server/next.ts
@@ -3,6 +3,7 @@ import type { NodeRequestHandler } from './next-server'
 import type { UrlWithParsedQuery } from 'url'
 
 import './node-polyfill-fetch'
+import React from 'react'
 import { default as Server } from './next-server'
 import * as log from '../build/output/log'
 import loadConfig from './config'
@@ -182,10 +183,7 @@ function createServer(options: NextServerOptions): NextServer {
     )
   }
 
-  // Make sure env of custom server is overridden.
-  // Use dynamic require to make sure it's executed in it's own context.
-  const ReactDOMServer = require('react-dom/server')
-  const shouldUseReactRoot = !!ReactDOMServer.renderToPipeableStream
+  const shouldUseReactRoot = parseInt(React.version) >= 18
   if (shouldUseReactRoot) {
     ;(process.env as any).__NEXT_REACT_ROOT = 'true'
   }

diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx
@@ -88,9 +88,10 @@ let tryGetPreviewData: typeof import('./api-utils/node').tryGetPreviewData
 let warn: typeof import('../build/output/log').warn
 
 const DOCTYPE = '<!DOCTYPE html>'
-const ReactDOMServer = process.env.__NEXT_REACT_ROOT
-  ? require('react-dom/server.browser')
-  : require('react-dom/server')
+const ReactDOMServer =
+  parseInt(React.version) >= 18
+    ? require('react-dom/server.browser')
+    : require('react-dom/server')
 
 if (process.env.NEXT_RUNTIME !== 'edge') {
   require('./node-polyfill-web-streams')

diff --git a/test/production/react-18-streaming-ssr/custom-server/next.config.js b/test/production/react-18-streaming-ssr/custom-server/next.config.js
@@ -0,0 +1,5 @@
+module.exports = {
+  experimental: {
+    serverComponents: true,
+  },
+}
diff --git a/test/production/react-18-streaming-ssr/custom-server/pages/index.server.js b/test/production/react-18-streaming-ssr/custom-server/pages/index.server.js
@@ -0,0 +1,7 @@
+export default function Page() {
+  return <p>streaming</p>
+}
+
+export async function getServerSideProps() {
+  return { props: {} }
+}
diff --git a/test/production/react-18-streaming-ssr/custom-server/server.js b/test/production/react-18-streaming-ssr/custom-server/server.js
@@ -0,0 +1,44 @@
+const NextServer = require('next/dist/server/next-server').default
+const defaultNextConfig =
+  require('next/dist/server/config-shared').defaultConfig
+const http = require('http')
+
+process.on('SIGTERM', () => process.exit(0))
+process.on('SIGINT', () => process.exit(0))
+
+let handler
+
+const server = http.createServer(async (req, res) => {
+  try {
+    await handler(req, res)
+  } catch (err) {
+    console.error(err)
+    res.statusCode = 500
+    res.end('internal server error')
+  }
+})
+const currentPort = parseInt(process.env.PORT, 10) || 3000
+
+server.listen(currentPort, (err) => {
+  if (err) {
+    console.error('Failed to start server', err)
+    process.exit(1)
+  }
+  const nextServer = new NextServer({
+    hostname: 'localhost',
+    port: currentPort,
+    customServer: true,
+    dev: false,
+    conf: {
+      ...defaultNextConfig,
+      distDir: '.next',
+      experimental: {
+        ...defaultNextConfig.experimental,
+        serverComponents: true,
+      },
+    },
+  })
+  handler = nextServer.getRequestHandler()
+
+  console.log('Listening on port', currentPort)
+})
diff --git a/test/production/react-18-streaming-ssr/index.test.ts b/test/production/react-18-streaming-ssr/index.test.ts
@@ -1,8 +1,20 @@
-import { createNext } from 'e2e-utils'
+import { join } from 'path'
+import { createNext, FileRef } from 'e2e-utils'
 import { NextInstance } from 'test/lib/next-modes/base'
-import { fetchViaHTTP, renderViaHTTP } from 'next-test-utils'
-
-describe('react 18 streaming SSR in minimal mode', () => {
+import {
+  fetchViaHTTP,
+  findPort,
+  initNextServerScript,
+  killApp,
+  renderViaHTTP,
+} from 'next-test-utils'
+
+const react18Deps = {
+  react: '^18.0.0',
+  'react-dom': '^18.0.0',
+}
+
+describe('react 18 streaming SSR and RSC in minimal mode', () => {
   let next: NextInstance
 
   beforeAll(async () => {
@@ -12,13 +24,15 @@ describe('react 18 streaming SSR in minimal mode', () => {
       files: {
         'pages/index.server.js': `
           export default function Page() {
-            return <p>static streaming</p>
+            return <p>streaming</p>
+          }
+          export async function getServerSideProps() {
+            return { props: {} }
           }
         `,
       },
       nextConfig: {
         experimental: {
-          reactRoot: true,
           serverComponents: true,
           runtime: 'nodejs',
         },
@@ -40,10 +54,7 @@ describe('react 18 streaming SSR in minimal mode', () => {
           return config
         },
       },
-      dependencies: {
-        react: '18.1.0',
-        'react-dom': '18.1.0',
-      },
+      dependencies: react18Deps,
     })
   })
   afterAll(() => {
@@ -58,7 +69,7 @@ describe('react 18 streaming SSR in minimal mode', () => {
 
   it('should generate html response by streaming correctly', async () => {
     const html = await renderViaHTTP(next.url, '/')
-    expect(html).toContain('static streaming')
+    expect(html).toContain('streaming')
   })
 
   it('should have generated a static 404 page', async () => {
@@ -76,49 +87,10 @@ describe('react 18 streaming SSR with custom next configs', () => {
   beforeAll(async () => {
     next = await createNext({
       files: {
-        'pages/index.js': `
-          export default function Page() {
-            return (
-              <div>
-                <style jsx>{\`p { color: blue } \`}</style>
-                <p>index</p>
-              </div>
-            )
-          }
-        `,
-        'pages/hello.js': `
-          import Link from 'next/link'
-
-          export default function Page() {
-            return (
-              <div>
-                <p>hello nextjs</p>
-                <Link href='/'><a>home></a></Link>
-              </div>
-            )
-          }
-        `,
-        'pages/multi-byte.js': `
-          export default function Page() {
-            return (
-              <div>
-                <p>{"マルチバイト".repeat(28)}</p>
-              </div>
-            );
-          }
-        `,
-      },
-      nextConfig: {
-        trailingSlash: true,
-        experimental: {
-          reactRoot: true,
-          runtime: 'edge',
-        },
-      },
-      dependencies: {
-        react: '18.1.0',
-        'react-dom': '18.1.0',
+        pages: new FileRef(join(__dirname, 'streaming-ssr/pages')),
       },
+      nextConfig: require(join(__dirname, 'streaming-ssr/next.config.js')),
+      dependencies: react18Deps,
       installCommand: 'npm install',
     })
   })
@@ -151,3 +123,43 @@ describe('react 18 streaming SSR with custom next configs', () => {
     expect(html).toContain('マルチバイト'.repeat(28))
   })
 })
+
+describe('react 18 streaming SSR and RSC with custom server', () => {
+  let next
+  let server
+  let appPort
+  beforeAll(async () => {
+    next = await createNext({
+      files: {
+        pages: new FileRef(join(__dirname, 'custom-server/pages')),
+        'server.js': new FileRef(join(__dirname, 'custom-server/server.js')),
+      },
+      nextConfig: require(join(__dirname, 'custom-server/next.config.js')),
+      dependencies: react18Deps,
+    })
+    await next.stop()
+
+    const testServer = join(next.testDir, 'server.js')
+    appPort = await findPort()
+    server = await initNextServerScript(
+      testServer,
+      /Listening/,
+      {
+        ...process.env,
+        PORT: appPort,
+      },
+      undefined,
+      {
+        cwd: next.testDir,
+      }
+    )
+  })
+  afterAll(async () => {
+    await next.destroy()
+    if (server) await killApp(server)
+  })
+  it('should render rsc correctly under custom server', async () => {
+    const html = await renderViaHTTP(appPort, '/')
+    expect(html).toContain('streaming')
+  })
+})
diff --git a/test/production/react-18-streaming-ssr/streaming-ssr/next.config.js b/test/production/react-18-streaming-ssr/streaming-ssr/next.config.js
@@ -0,0 +1,6 @@
+module.exports = {
+  trailingSlash: true,
+  experimental: {
+    runtime: 'edge',
+  },
+}
diff --git a/test/production/react-18-streaming-ssr/streaming-ssr/pages/hello.js b/test/production/react-18-streaming-ssr/streaming-ssr/pages/hello.js
@@ -0,0 +1,14 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <div>
+      <p>hello nextjs</p>
+      <Link href="/">
+        <a>home</a>
+      </Link>
+    </div>
+  )
+}
+
+export const config = { runtime: 'edge' }
diff --git a/test/production/react-18-streaming-ssr/streaming-ssr/pages/index.js b/test/production/react-18-streaming-ssr/streaming-ssr/pages/index.js
@@ -0,0 +1,14 @@
+export default function Page() {
+  return (
+    <div>
+      <style jsx>{`
+        p {
+          color: blue;
+        }
+      `}</style>
+      <p>index</p>
+    </div>
+  )
+}
+
+export const config = { runtime: 'edge' }
diff --git a/test/production/react-18-streaming-ssr/streaming-ssr/pages/multi-byte.js b/test/production/react-18-streaming-ssr/streaming-ssr/pages/multi-byte.js
@@ -0,0 +1,9 @@
+export default function Page() {
+  return (
+    <div>
+      <p>{'マルチバイト'.repeat(28)}</p>
+    </div>
+  )
+}
+
+export const config = { runtime: 'edge' }