Fix uncaught error in getInitialProps when runtime is set to `nodej…

…s` (#34228) This PR ensures that the test "should render 500 error correctly" doesn't break when `runtime` is set to `nodejs` with `serverComponents` enabled. This test case is now moved to the "basic" suite to ensure it doesn't break in both runtimes. And "should not bundle external imports into client builds for RSC" is enabled for the `nodejs` runtime too. ## Bug - [ ] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint`
vercel · Feb 11, 2022 · 931666d · 931666d
1 parent 477134d
commit 931666d
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 57 deletions.
diff --git a/packages/next/build/webpack/loaders/next-flight-server-loader.ts b/packages/next/build/webpack/loaders/next-flight-server-loader.ts
@@ -104,6 +104,12 @@ async function parseImportsInfo(
         }
         break
       }
+      case 'ExportDefaultExpression':
+        const exp = node.expression
+        if (exp.type === 'Identifier') {
+          defaultExportName = exp.value
+        }
+        break
       default:
         break
     }
@@ -157,11 +163,18 @@ export default async function transformSource(
    */
 
   const noop = `export const __rsc_noop__=()=>{${imports.join(';')}}`
-  const defaultExportNoop = isClientCompilation
-    ? `export default function ${defaultExportName}(){}\n${defaultExportName}.__next_rsc__=1;`
-    : defaultExportName
-    ? `${defaultExportName}.__next_rsc__=1;${defaultExportName}.__webpack_require__=__webpack_require__;`
-    : ''
+
+  let defaultExportNoop = ''
+  if (isClientCompilation) {
+    defaultExportNoop = `export default function ${
+      defaultExportName || 'ServerComponent'
+    }(){}\n${defaultExportName || 'ServerComponent'}.__next_rsc__=1;`
+  } else {
+    if (defaultExportName) {
+      // It's required to have the default export for pages. For other components, it's fine to leave it as is.
+      defaultExportNoop = `${defaultExportName}.__next_rsc__=1;${defaultExportName}.__webpack_require__=__webpack_require__;`
+    }
+  }
 
   const transformed = transformedSource + '\n' + noop + '\n' + defaultExportNoop
 

diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx
@@ -467,9 +467,14 @@ export async function renderToHTML(
 
   const hasConcurrentFeatures = !!runtime
 
-  const isServerComponent = !!serverComponentManifest && hasConcurrentFeatures
   const OriginalComponent = renderOpts.Component
 
+  // We don't need to opt-into the flight inlining logic if the page isn't a RSC.
+  const isServerComponent =
+    !!serverComponentManifest &&
+    hasConcurrentFeatures &&
+    (OriginalComponent as any).__next_rsc__
+
   let Component: React.ComponentType<{}> | ((props: any) => JSX.Element) =
     renderOpts.Component
   let serverComponentsInlinedTransformStream: TransformStream<any, any> | null =

diff --git a/test/integration/react-streaming-and-server-components/test/basic.js b/test/integration/react-streaming-and-server-components/test/basic.js
@@ -1,7 +1,7 @@
 import webdriver from 'next-webdriver'
 import { renderViaHTTP } from 'next-test-utils'
 
-export default async function basic(context) {
+export default async function basic(context, { env }) {
   it('should render 404 error correctly', async () => {
     const path404HTML = await renderViaHTTP(context.appPort, '/404')
     const pathNotFoundHTML = await renderViaHTTP(context.appPort, '/not-found')
@@ -35,4 +35,15 @@ export default async function basic(context) {
 
     expect(hydrationContent).toBe('custom-404-pagenext_streaming_data')
   })
+
+  it('should render 500 error correctly', async () => {
+    const path500HTML = await renderViaHTTP(context.appPort, '/err')
+
+    if (env === 'dev') {
+      // In dev mode it should show the error popup.
+      expect(path500HTML).toContain('Error: oops')
+    } else {
+      expect(path500HTML).toContain('custom-500-page')
+    }
+  })
 }
diff --git a/test/integration/react-streaming-and-server-components/test/index.test.js b/test/integration/react-streaming-and-server-components/test/index.test.js
@@ -4,31 +4,28 @@ import { join } from 'path'
 import fs from 'fs-extra'
 import webdriver from 'next-webdriver'
 
-import {
-  File,
-  fetchViaHTTP,
-  findPort,
-  killApp,
-  renderViaHTTP,
-} from 'next-test-utils'
+import { fetchViaHTTP, findPort, killApp, renderViaHTTP } from 'next-test-utils'
 
-import { nextBuild, nextStart, nextDev } from './utils'
+import {
+  nextBuild,
+  nextStart,
+  nextDev,
+  appDir,
+  nativeModuleTestAppDir,
+  distDir,
+  documentPage,
+  appPage,
+  appServerPage,
+  error500Page,
+  nextConfig,
+} from './utils'
 
 import css from './css'
 import rsc from './rsc'
 import streaming from './streaming'
 import basic from './basic'
 import functions from './functions'
 
-const appDir = join(__dirname, '../app')
-const nativeModuleTestAppDir = join(__dirname, '../unsupported-native-module')
-const distDir = join(__dirname, '../app/.next')
-const documentPage = new File(join(appDir, 'pages/_document.jsx'))
-const appPage = new File(join(appDir, 'pages/_app.js'))
-const appServerPage = new File(join(appDir, 'pages/_app.server.js'))
-const error500Page = new File(join(appDir, 'pages/500.js'))
-const nextConfig = new File(join(appDir, 'next.config.js'))
-
 const documentWithGip = `
 import { Html, Head, Main, NextScript } from 'next/document'
 
@@ -153,14 +150,9 @@ describe('Edge runtime - prod', () => {
     expect(html).toContain('foo.client')
   })
 
-  it('should render 500 error correctly', async () => {
-    const path500HTML = await renderViaHTTP(context.appPort, '/err')
-    expect(path500HTML).toContain('custom-500-page')
-  })
-
-  basic(context)
-  rsc(context, 'edge')
+  basic(context, { env: 'prod' })
   streaming(context)
+  rsc(context, { runtime: 'edge', env: 'prod' })
 })
 
 describe('Edge runtime - dev', () => {
@@ -185,35 +177,16 @@ describe('Edge runtime - dev', () => {
     expect(content).toMatchInlineSnapshot('"foo.client"')
   })
 
-  it('should not bundle external imports into client builds for RSC', async () => {
-    const html = await renderViaHTTP(context.appPort, '/external-imports')
-    expect(html).toContain('date:')
-
-    const distServerDir = join(distDir, 'static', 'chunks', 'pages')
-    const bundle = fs
-      .readFileSync(join(distServerDir, 'external-imports.js'))
-      .toString()
-
-    expect(bundle).not.toContain('moment')
-  })
-
-  it('should render 500 error correctly', async () => {
-    const path500HTML = await renderViaHTTP(context.appPort, '/err')
-
-    // In dev mode it should show the error popup.
-    expect(path500HTML).toContain('Error: oops')
-  })
-
-  basic(context)
-  rsc(context, 'edge')
+  basic(context, { env: 'dev' })
   streaming(context)
+  rsc(context, { runtime: 'edge', env: 'dev' })
 })
 
 const nodejsRuntimeBasicSuite = {
   runTests: (context, env) => {
-    basic(context)
+    basic(context, { env })
     streaming(context)
-    rsc(context, 'nodejs')
+    rsc(context, { runtime: 'nodejs' })
 
     if (env === 'prod') {
       it('should generate middleware SSR manifests for Node.js', async () => {
@@ -241,8 +214,14 @@ const nodejsRuntimeBasicSuite = {
       })
     }
   },
-  beforeAll: () => nextConfig.replace("runtime: 'edge'", "runtime: 'nodejs'"),
-  afterAll: () => nextConfig.restore(),
+  beforeAll: () => {
+    error500Page.write(page500)
+    nextConfig.replace("runtime: 'edge'", "runtime: 'nodejs'")
+  },
+  afterAll: () => {
+    error500Page.delete()
+    nextConfig.restore()
+  },
 }
 
 const customAppPageSuite = {

diff --git a/test/integration/react-streaming-and-server-components/test/rsc.js b/test/integration/react-streaming-and-server-components/test/rsc.js
@@ -2,13 +2,17 @@
 import webdriver from 'next-webdriver'
 import cheerio from 'cheerio'
 import { renderViaHTTP, check } from 'next-test-utils'
+import { join } from 'path'
+import fs from 'fs-extra'
+
+import { distDir } from './utils'
 
 function getNodeBySelector(html, selector) {
   const $ = cheerio.load(html)
   return $(selector)
 }
 
-export default function (context, runtime) {
+export default function (context, { runtime, env }) {
   it('should render server components correctly', async () => {
     const homeHTML = await renderViaHTTP(context.appPort, '/', null, {
       headers: {
@@ -72,6 +76,22 @@ export default function (context, runtime) {
     })
   }
 
+  // For prod build, the directory contains the build ID so it's not deterministic.
+  // Only enable it for dev for now.
+  if (env === 'dev') {
+    it('should not bundle external imports into client builds for RSC', async () => {
+      const html = await renderViaHTTP(context.appPort, '/external-imports')
+      expect(html).toContain('date:')
+
+      const distServerDir = join(distDir, 'static', 'chunks', 'pages')
+      const bundle = fs
+        .readFileSync(join(distServerDir, 'external-imports.js'))
+        .toString()
+
+      expect(bundle).not.toContain('moment')
+    })
+  }
+
   it('should handle multiple named exports correctly', async () => {
     const clientExportsHTML = await renderViaHTTP(
       context.appPort,

diff --git a/test/integration/react-streaming-and-server-components/test/utils.js b/test/integration/react-streaming-and-server-components/test/utils.js
@@ -1,12 +1,25 @@
 import { join } from 'path'
 import {
+  File,
   launchApp,
   nextBuild as _nextBuild,
   nextStart as _nextStart,
 } from 'next-test-utils'
 
 const nodeArgs = ['-r', join(__dirname, '../../react-18/test/require-hook.js')]
 
+export const appDir = join(__dirname, '../app')
+export const nativeModuleTestAppDir = join(
+  __dirname,
+  '../unsupported-native-module'
+)
+export const distDir = join(__dirname, '../app/.next')
+export const documentPage = new File(join(appDir, 'pages/_document.jsx'))
+export const appPage = new File(join(appDir, 'pages/_app.js'))
+export const appServerPage = new File(join(appDir, 'pages/_app.server.js'))
+export const error500Page = new File(join(appDir, 'pages/500.js'))
+export const nextConfig = new File(join(appDir, 'next.config.js'))
+
 export async function nextBuild(dir, options) {
   return await _nextBuild(dir, [], {
     ...options,