From c4d00df5ba0c3aed7a8f91739dab4ac167c1b729 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 10 Feb 2022 16:40:01 +0100 Subject: [PATCH 1/2] Fix page navigation of rsc nodejs --- packages/next/client/page-loader.ts | 2 +- .../test/basic.js | 11 +++- .../test/index.test.js | 53 +++---------------- .../test/rsc.js | 17 +++--- 4 files changed, 27 insertions(+), 56 deletions(-) diff --git a/packages/next/client/page-loader.ts b/packages/next/client/page-loader.ts index d4119a3b2767c..0ec4e6945827b 100644 --- a/packages/next/client/page-loader.ts +++ b/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( diff --git a/test/integration/react-streaming-and-server-components/test/basic.js b/test/integration/react-streaming-and-server-components/test/basic.js index d9fe204a5cf79..64629a9b356e4 100644 --- a/test/integration/react-streaming-and-server-components/test/basic.js +++ b/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,12 @@ 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.eval( + `document.querySelector('#__next').textContent` + ) + expect(hydrationContent).toBe('custom-404-pagenext_streaming_data') + }) } 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 index c0733b50468b5..6777fa461c88a 100644 --- a/test/integration/react-streaming-and-server-components/test/index.test.js +++ b/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 ( - - custom-404-page - - - ) -} -` - 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 () => { diff --git a/test/integration/react-streaming-and-server-components/test/rsc.js b/test/integration/react-streaming-and-server-components/test/rsc.js index 7fe0bfc227163..674c8201b9a59 100644 --- a/test/integration/react-streaming-and-server-components/test/rsc.js +++ b/test/integration/react-streaming-and-server-components/test/rsc.js @@ -3,7 +3,7 @@ import webdriver from 'next-webdriver' import cheerio from 'cheerio' import { renderViaHTTP, check } from 'next-test-utils' -export default function (context) { +export default function (context, runtime) { it('should render server components correctly', async () => { const homeHTML = await renderViaHTTP(context.appPort, '/', null, { headers: { @@ -52,13 +52,16 @@ 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 $ = cheerio.load(imageHTML) + const imageTag = $('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( From 4c9ff37584321178f77eb5812a8e1189f26a6cc7 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 10 Feb 2022 20:54:52 +0100 Subject: [PATCH 2/2] update test --- packages/next/client/index.tsx | 2 +- packages/next/server/base-server.ts | 6 ++- packages/next/server/render.tsx | 3 +- .../app/pages/404.js | 3 +- .../test/basic.js | 5 +- .../test/rsc.js | 51 +++++++++++++++---- 6 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 0f2c6ccbf33bd..8115aab7519d6 100644 --- a/packages/next/client/index.tsx +++ b/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() diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index a0ff7eb45b242..180768e6d354a 100644 --- a/packages/next/server/base-server.ts +++ b/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 } diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 7976f6c7eda06..0d77d18f5f0dc 100644 --- a/packages/next/server/render.tsx +++ b/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, diff --git a/test/integration/react-streaming-and-server-components/app/pages/404.js b/test/integration/react-streaming-and-server-components/app/pages/404.js index fd264d6b7ed87..9b643c13f00b9 100644 --- a/test/integration/react-streaming-and-server-components/app/pages/404.js +++ b/test/integration/react-streaming-and-server-components/app/pages/404.js @@ -17,8 +17,7 @@ function Data() { export default function Page404() { return ( - custom-404-page -
+ custom-404-page ) diff --git a/test/integration/react-streaming-and-server-components/test/basic.js b/test/integration/react-streaming-and-server-components/test/basic.js index 64629a9b356e4..468108ffe94b1 100644 --- a/test/integration/react-streaming-and-server-components/test/basic.js +++ b/test/integration/react-streaming-and-server-components/test/basic.js @@ -31,9 +31,8 @@ export default async function basic(context) { it('should handle suspense error page correctly (node stream)', async () => { const browser = await webdriver(context.appPort, '/404') - const hydrationContent = await browser.eval( - `document.querySelector('#__next').textContent` - ) + const hydrationContent = await browser.waitForElementByCss('#__next').text() + expect(hydrationContent).toBe('custom-404-pagenext_streaming_data') }) } diff --git a/test/integration/react-streaming-and-server-components/test/rsc.js b/test/integration/react-streaming-and-server-components/test/rsc.js index 674c8201b9a59..77cee8d228c47 100644 --- a/test/integration/react-streaming-and-server-components/test/rsc.js +++ b/test/integration/react-streaming-and-server-components/test/rsc.js @@ -3,6 +3,11 @@ import webdriver from 'next-webdriver' import cheerio from 'cheerio' import { renderViaHTTP, check } from 'next-test-utils' +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, { @@ -29,8 +34,10 @@ export default function (context, runtime) { 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') @@ -56,8 +63,10 @@ export default function (context, runtime) { if (runtime === 'edge') { 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') + const imageTag = getNodeBySelector( + imageHTML, + 'div[hidden] > span > span > img' + ) expect(imageTag.attr('src')).toContain('data:image') }) @@ -68,12 +77,18 @@ export default function (context, runtime) { 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') @@ -86,4 +101,22 @@ export default function (context, runtime) { 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) + }) }