Fix reuse of inline flight response and 404 for RSC in node runtime (#…

…34202)

### Fixes

* We need give value for `__flight__` query or it will lost in node runtime
* Match the cache key of flight response (`path + search`, `route, `id`)
* Flight response should return `200` instead of `404` for 404 page

### Tests

* Run rsc for node runtime test suite
* Add more cases for 404

packages/next/client/index.tsx

@@ -744,7 +744,7 @@ if (process.env.__NEXT_RSC) {
     let response = rscCache.get(cacheKey)
     if (response) return response
 
-    const bufferCacheKey = cacheKey + ',' + id
+    const bufferCacheKey = cacheKey + ',' + router.route + ',' + id
     if (serverDataBuffer.has(bufferCacheKey)) {
       const t = new TransformStream()
       const writer = t.writable.getWriter()

packages/next/client/page-loader.ts

@@ -148,7 +148,7 @@ export default class PageLoader {
 
     const getHrefForSlug = (path: string) => {
       if (rsc) {
-        return path + search + (search ? `&` : '?') + '__flight__'
+        return path + search + (search ? `&` : '?') + '__flight__=1'
       }
 
       const dataRoute = getAssetPathFromRoute(

packages/next/server/base-server.ts

@@ -1126,9 +1126,13 @@ export default abstract class Server {
     // Toggle whether or not this is a Data request
     const isDataReq = !!query._nextDataReq && (isSSG || hasServerProps)
     delete query._nextDataReq
+    // Don't delete query.__flight__ yet, it still needs to be used in renderToHTML later
+    const isFlightRequest = Boolean(
+      this.serverComponentManifest && query.__flight__
+    )
 
     // we need to ensure the status code if /404 is visited directly
-    if (is404Page && !isDataReq) {
+    if (is404Page && !isDataReq && !isFlightRequest) {
       res.statusCode = 404
     }
 

packages/next/server/render.tsx

@@ -477,8 +477,9 @@ export async function renderToHTML(
 
   if (isServerComponent) {
     serverComponentsInlinedTransformStream = new TransformStream()
+    const search = stringifyQuery(query)
     Component = createServerComponentRenderer(App, OriginalComponent, {
-      cachePrefix: pathname + '?' + stringifyQuery(query),
+      cachePrefix: pathname + (search ? `?${search}` : ''),
       transformStream: serverComponentsInlinedTransformStream,
       serverComponentManifest,
       runtime,

test/integration/react-streaming-and-server-components/app/pages/404.js

@@ -17,8 +17,7 @@ function Data() {
 export default function Page404() {
   return (
     <Suspense fallback={null}>
-      custom-404-page
-      <br />
+      <span id="text">custom-404-page</span>
       <Data />
     </Suspense>
   )

test/integration/react-streaming-and-server-components/test/basic.js

@@ -1,6 +1,7 @@
+import webdriver from 'next-webdriver'
 import { renderViaHTTP } from 'next-test-utils'
 
-export default async function basic(context, env) {
+export default async function basic(context) {
   it('should render 404 error correctly', async () => {
     const path404HTML = await renderViaHTTP(context.appPort, '/404')
     const pathNotFoundHTML = await renderViaHTTP(context.appPort, '/not-found')
@@ -27,4 +28,11 @@ export default async function basic(context, env) {
     const res = await renderViaHTTP(context.appPort, '/api/ping')
     expect(res).toContain('pong')
   })
+
+  it('should handle suspense error page correctly (node stream)', async () => {
+    const browser = await webdriver(context.appPort, '/404')
+    const hydrationContent = await browser.waitForElementByCss('#__next').text()
+
+    expect(hydrationContent).toBe('custom-404-pagenext_streaming_data')
+  })
 }

test/integration/react-streaming-and-server-components/test/index.test.js

@@ -27,7 +27,6 @@ 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 error404Page = new File(join(appDir, 'pages/404.js'))
 const nextConfig = new File(join(appDir, 'next.config.js'))
 
 const documentWithGip = `
@@ -73,33 +72,6 @@ export default function Page500() {
 }
 `
 
-const suspense404 = `
-import { Suspense } from 'react'
-
-let result
-let promise
-function Data() {
-  if (result) return result
-  if (!promise)
-    promise = new Promise((res) => {
-      setTimeout(() => {
-        result = 'next_streaming_data'
-        res()
-      }, 500)
-    })
-  throw promise
-}
-
-export default function Page404() {
-  return (
-    <Suspense fallback={null}>
-      custom-404-page
-      <Data />
-    </Suspense>
-  )
-}
-`
-
 describe('Edge runtime - basic', () => {
   it('should warn user for experimental risk with server components', async () => {
     const edgeRuntimeWarning =
@@ -116,20 +88,6 @@ describe('Edge runtime - basic', () => {
     const { stderr } = await nextBuild(nativeModuleTestAppDir)
     expect(stderr).toContain(fsImportedErrorMessage)
   })
-
-  it('should handle suspense error page correctly (node stream)', async () => {
-    error404Page.write(suspense404)
-    const appPort = await findPort()
-    await nextBuild(appDir)
-    await nextStart(appDir, appPort)
-    const browser = await webdriver(appPort, '/404')
-    const hydrationContent = await browser.eval(
-      `document.querySelector('#__next').textContent`
-    )
-    expect(hydrationContent).toBe('custom-404-pagenext_streaming_data')
-
-    error404Page.restore()
-  })
 })
 
 describe('Edge runtime - prod', () => {
@@ -200,8 +158,8 @@ describe('Edge runtime - prod', () => {
     expect(path500HTML).toContain('custom-500-page')
   })
 
-  basic(context, 'prod')
-  rsc(context)
+  basic(context)
+  rsc(context, 'edge')
   streaming(context)
 })
 
@@ -246,15 +204,16 @@ describe('Edge runtime - dev', () => {
     expect(path500HTML).toContain('Error: oops')
   })
 
-  basic(context, 'dev')
-  rsc(context)
+  basic(context)
+  rsc(context, 'edge')
   streaming(context)
 })
 
 const nodejsRuntimeBasicSuite = {
   runTests: (context, env) => {
-    basic(context, env)
+    basic(context)
     streaming(context)
+    rsc(context, 'nodejs')
 
     if (env === 'prod') {
       it('should generate middleware SSR manifests for Node.js', async () => {

test/integration/react-streaming-and-server-components/test/rsc.js

@@ -3,7 +3,12 @@ import webdriver from 'next-webdriver'
 import cheerio from 'cheerio'
 import { renderViaHTTP, check } from 'next-test-utils'
 
-export default function (context) {
+function getNodeBySelector(html, selector) {
+  const $ = cheerio.load(html)
+  return $(selector)
+}
+
+export default function (context, runtime) {
   it('should render server components correctly', async () => {
     const homeHTML = await renderViaHTTP(context.appPort, '/', null, {
       headers: {
@@ -29,8 +34,10 @@ export default function (context) {
 
   it('should support next/link in server components', async () => {
     const linkHTML = await renderViaHTTP(context.appPort, '/next-api/link')
-    const $ = cheerio.load(linkHTML)
-    const linkText = $('div[hidden] > a[href="/"]').text()
+    const linkText = getNodeBySelector(
+      linkHTML,
+      'div[hidden] > a[href="/"]'
+    ).text()
 
     expect(linkText).toContain('go home')
 
@@ -52,25 +59,36 @@ export default function (context) {
     expect(await browser.eval('window.beforeNav')).toBe(1)
   })
 
-  it('should suspense next/image in server components', async () => {
-    const imageHTML = await renderViaHTTP(context.appPort, '/next-api/image')
-    const $ = cheerio.load(imageHTML)
-    const imageTag = $('div[hidden] > span > span > img')
+  // Disable next/image for nodejs runtime temporarily
+  if (runtime === 'edge') {
+    it('should suspense next/image in server components', async () => {
+      const imageHTML = await renderViaHTTP(context.appPort, '/next-api/image')
+      const imageTag = getNodeBySelector(
+        imageHTML,
+        'div[hidden] > span > span > img'
+      )
 
-    expect(imageTag.attr('src')).toContain('data:image')
-  })
+      expect(imageTag.attr('src')).toContain('data:image')
+    })
+  }
 
   it('should handle multiple named exports correctly', async () => {
     const clientExportsHTML = await renderViaHTTP(
       context.appPort,
       '/client-exports'
     )
-    const $clientExports = cheerio.load(clientExportsHTML)
-    expect($clientExports('div[hidden] > div > #named-exports').text()).toBe(
-      'abcde'
-    )
+
+    expect(
+      getNodeBySelector(
+        clientExportsHTML,
+        'div[hidden] > div > #named-exports'
+      ).text()
+    ).toBe('abcde')
     expect(
-      $clientExports('div[hidden] > div > #default-exports-arrow').text()
+      getNodeBySelector(
+        clientExportsHTML,
+        'div[hidden] > div > #default-exports-arrow'
+      ).text()
     ).toBe('client-default-export-arrow')
 
     const browser = await webdriver(context.appPort, '/client-exports')
@@ -83,4 +101,22 @@ export default function (context) {
     expect(textNamedExports).toBe('abcde')
     expect(textDefaultExportsArrow).toBe('client-default-export-arrow')
   })
+
+  it('should handle 404 requests and missing routes correctly', async () => {
+    const id = '#text'
+    const content = 'custom-404-page'
+    const page404HTML = await renderViaHTTP(context.appPort, '/404')
+    const pageUnknownHTML = await renderViaHTTP(context.appPort, '/no.where')
+
+    const page404Browser = await webdriver(context.appPort, '/404')
+    const pageUnknownBrowser = await webdriver(context.appPort, '/no.where')
+
+    expect(await page404Browser.waitForElementByCss(id).text()).toBe(content)
+    expect(await pageUnknownBrowser.waitForElementByCss(id).text()).toBe(
+      content
+    )
+
+    expect(getNodeBySelector(page404HTML, id).text()).toBe(content)
+    expect(getNodeBySelector(pageUnknownHTML, id).text()).toBe(content)
+  })
 }