From 8f8a16691459799cf4c3e3bc694c7f922ea35153 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 7 Aug 2023 18:39:03 +0200 Subject: [PATCH 1/2] Fix not-found rendering in production with edge --- packages/next/src/server/base-server.ts | 12 ++++- packages/next/src/server/web-server.ts | 5 ++ .../{ => basic}/app/dynamic/[id]/not-found.js | 0 .../{ => basic}/app/dynamic/[id]/page.js | 0 .../not-found/{ => basic}/app/dynamic/page.js | 0 .../not-found/{ => basic}/app/layout.js | 0 .../not-found/{ => basic}/app/not-found.js | 0 .../app-dir/not-found/{ => basic}/app/page.js | 0 .../index.test.ts} | 33 +++++------- .../not-found/conflict-route/app/layout.js | 16 ++++++ .../not-found/conflict-route/app/not-found.js | 11 ++++ .../app/not-found/not-found.js | 0 .../app/not-found/page.js | 0 .../not-found/conflict-route/app/page.js | 3 ++ .../not-found/conflict-route/index.test.ts | 50 +++++++++++++++++++ test/e2e/app-dir/not-found/next.config.js | 1 - 16 files changed, 107 insertions(+), 24 deletions(-) rename test/e2e/app-dir/not-found/{ => basic}/app/dynamic/[id]/not-found.js (100%) rename test/e2e/app-dir/not-found/{ => basic}/app/dynamic/[id]/page.js (100%) rename test/e2e/app-dir/not-found/{ => basic}/app/dynamic/page.js (100%) rename test/e2e/app-dir/not-found/{ => basic}/app/layout.js (100%) rename test/e2e/app-dir/not-found/{ => basic}/app/not-found.js (100%) rename test/e2e/app-dir/not-found/{ => basic}/app/page.js (100%) rename test/e2e/app-dir/not-found/{not-found.test.ts => basic/index.test.ts} (76%) create mode 100644 test/e2e/app-dir/not-found/conflict-route/app/layout.js create mode 100644 test/e2e/app-dir/not-found/conflict-route/app/not-found.js rename test/e2e/app-dir/not-found/{ => conflict-route}/app/not-found/not-found.js (100%) rename test/e2e/app-dir/not-found/{ => conflict-route}/app/not-found/page.js (100%) create mode 100644 test/e2e/app-dir/not-found/conflict-route/app/page.js create mode 100644 test/e2e/app-dir/not-found/conflict-route/index.test.ts delete mode 100644 test/e2e/app-dir/not-found/next.config.js diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 33f8887c0d6f6..0c2a79233fb61 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1411,7 +1411,10 @@ export default abstract class Server { { req, res, pathname, renderOpts: opts }: RequestContext, { components, query }: FindComponentsResult ): Promise { - const is404Page = pathname === '/404' + const is404Page = + // For edge runtime 404 page, /_not-found needs to be treated as 404 page + (process.env.NEXT_RUNTIME === 'edge' && pathname === '/_not-found') || + pathname === '/404' const is500Page = pathname === '/500' const isAppPath = components.isAppPath const hasServerProps = !!components.getServerSideProps @@ -1920,7 +1923,12 @@ export default abstract class Server { (req as NodeNextRequest).originalRequest ?? (req as WebNextRequest), (res as NodeNextResponse).originalResponse ?? (res as WebNextResponse), - { page: pathname, params: opts.params, query, renderOpts } + { + page: is404Page ? '/404' : pathname, + params: opts.params, + query, + renderOpts, + } ) } else { // If we didn't match a page, we should fallback to using the legacy diff --git a/packages/next/src/server/web-server.ts b/packages/next/src/server/web-server.ts index 7d44bf0991e12..9de400b22333b 100644 --- a/packages/next/src/server/web-server.ts +++ b/packages/next/src/server/web-server.ts @@ -330,6 +330,11 @@ export default class NextWebServer extends BaseServer { ) } + // For edge runtime if the pathname hit as /_not-found entrypoint, + // override the pathname to /404 for rendering + if (pathname === (renderOpts.dev ? '/not-found' : '/_not-found')) { + pathname = '/404' + } return renderToHTML( req as any, res as any, diff --git a/test/e2e/app-dir/not-found/app/dynamic/[id]/not-found.js b/test/e2e/app-dir/not-found/basic/app/dynamic/[id]/not-found.js similarity index 100% rename from test/e2e/app-dir/not-found/app/dynamic/[id]/not-found.js rename to test/e2e/app-dir/not-found/basic/app/dynamic/[id]/not-found.js diff --git a/test/e2e/app-dir/not-found/app/dynamic/[id]/page.js b/test/e2e/app-dir/not-found/basic/app/dynamic/[id]/page.js similarity index 100% rename from test/e2e/app-dir/not-found/app/dynamic/[id]/page.js rename to test/e2e/app-dir/not-found/basic/app/dynamic/[id]/page.js diff --git a/test/e2e/app-dir/not-found/app/dynamic/page.js b/test/e2e/app-dir/not-found/basic/app/dynamic/page.js similarity index 100% rename from test/e2e/app-dir/not-found/app/dynamic/page.js rename to test/e2e/app-dir/not-found/basic/app/dynamic/page.js diff --git a/test/e2e/app-dir/not-found/app/layout.js b/test/e2e/app-dir/not-found/basic/app/layout.js similarity index 100% rename from test/e2e/app-dir/not-found/app/layout.js rename to test/e2e/app-dir/not-found/basic/app/layout.js diff --git a/test/e2e/app-dir/not-found/app/not-found.js b/test/e2e/app-dir/not-found/basic/app/not-found.js similarity index 100% rename from test/e2e/app-dir/not-found/app/not-found.js rename to test/e2e/app-dir/not-found/basic/app/not-found.js diff --git a/test/e2e/app-dir/not-found/app/page.js b/test/e2e/app-dir/not-found/basic/app/page.js similarity index 100% rename from test/e2e/app-dir/not-found/app/page.js rename to test/e2e/app-dir/not-found/basic/app/page.js diff --git a/test/e2e/app-dir/not-found/not-found.test.ts b/test/e2e/app-dir/not-found/basic/index.test.ts similarity index 76% rename from test/e2e/app-dir/not-found/not-found.test.ts rename to test/e2e/app-dir/not-found/basic/index.test.ts index 56925ba5905bc..868059fb9cf13 100644 --- a/test/e2e/app-dir/not-found/not-found.test.ts +++ b/test/e2e/app-dir/not-found/basic/index.test.ts @@ -2,7 +2,7 @@ import { createNextDescribe } from 'e2e-utils' import { check } from 'next-test-utils' createNextDescribe( - 'app dir - not-found', + 'app dir - not-found - basic', { files: __dirname, skipDeployment: true, @@ -32,11 +32,6 @@ createNextDescribe( expect(await browser.elementByCss('#layout-nav').text()).toBe('Navbar') }) - it('should allow to have a valid /not-found route', async () => { - const html = await next.render('/not-found') - expect(html).toContain("I'm still a valid page") - }) - it('should match dynamic route not-found boundary correctly', async () => { const $dynamic = await next.render$('/dynamic') const $dynamicId = await next.render$('/dynamic/123') @@ -61,21 +56,17 @@ createNextDescribe( }, 'success') }) - // TODO: investigate isEdge case - if (!isEdge) { - it('should render the 404 page when the file is removed, and restore the page when re-added', async () => { - const browser = await next.browser('/') - await check(() => browser.elementByCss('h1').text(), 'My page') - await next.renameFile('./app/page.js', './app/foo.js') - await check( - () => browser.elementByCss('h1').text(), - 'This Is The Not Found Page' - ) - // TODO: investigate flakey behavior - // await next.renameFile('./app/foo.js', './app/page.js') - // await check(() => browser.elementByCss('h1').text(), 'My page') - }) - } + it('should render the 404 page when the file is removed, and restore the page when re-added', async () => { + const browser = await next.browser('/') + await check(() => browser.elementByCss('h1').text(), 'My page') + await next.renameFile('./app/page.js', './app/foo.js') + await check( + () => browser.elementByCss('h1').text(), + 'This Is The Not Found Page' + ) + await next.renameFile('./app/foo.js', './app/page.js') + await check(() => browser.elementByCss('h1').text(), 'My page') + }) } if (!isNextDev && !isEdge) { diff --git a/test/e2e/app-dir/not-found/conflict-route/app/layout.js b/test/e2e/app-dir/not-found/conflict-route/app/layout.js new file mode 100644 index 0000000000000..ba30dadf0ac5e --- /dev/null +++ b/test/e2e/app-dir/not-found/conflict-route/app/layout.js @@ -0,0 +1,16 @@ +export default function Layout({ children }) { + return ( + + + +
+ +
+
{children}
+
+ +
+ + + ) +} diff --git a/test/e2e/app-dir/not-found/conflict-route/app/not-found.js b/test/e2e/app-dir/not-found/conflict-route/app/not-found.js new file mode 100644 index 0000000000000..9cefbbcf6c820 --- /dev/null +++ b/test/e2e/app-dir/not-found/conflict-route/app/not-found.js @@ -0,0 +1,11 @@ +export default function NotFound() { + return ( + <> +

This Is The Not Found Page

+ +
{Date.now()}
+ + ) +} + +NotFound.displayName = 'NotFound' diff --git a/test/e2e/app-dir/not-found/app/not-found/not-found.js b/test/e2e/app-dir/not-found/conflict-route/app/not-found/not-found.js similarity index 100% rename from test/e2e/app-dir/not-found/app/not-found/not-found.js rename to test/e2e/app-dir/not-found/conflict-route/app/not-found/not-found.js diff --git a/test/e2e/app-dir/not-found/app/not-found/page.js b/test/e2e/app-dir/not-found/conflict-route/app/not-found/page.js similarity index 100% rename from test/e2e/app-dir/not-found/app/not-found/page.js rename to test/e2e/app-dir/not-found/conflict-route/app/not-found/page.js diff --git a/test/e2e/app-dir/not-found/conflict-route/app/page.js b/test/e2e/app-dir/not-found/conflict-route/app/page.js new file mode 100644 index 0000000000000..e5c1e42bcc385 --- /dev/null +++ b/test/e2e/app-dir/not-found/conflict-route/app/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

My page

+} diff --git a/test/e2e/app-dir/not-found/conflict-route/index.test.ts b/test/e2e/app-dir/not-found/conflict-route/index.test.ts new file mode 100644 index 0000000000000..ad934522a6e05 --- /dev/null +++ b/test/e2e/app-dir/not-found/conflict-route/index.test.ts @@ -0,0 +1,50 @@ +import { createNextDescribe } from 'e2e-utils' +import { check } from 'next-test-utils' + +createNextDescribe( + 'app dir - not-found - conflict route', + { + files: __dirname, + skipDeployment: true, + }, + ({ next }) => { + const runTests = () => { + it('should use the not-found page for non-matching routes', async () => { + const browser = await next.browser('/random-content') + expect(await browser.elementByCss('h1').text()).toContain( + 'This Is The Not Found Page' + ) + // should contain root layout content + expect(await browser.elementByCss('#layout-nav').text()).toBe('Navbar') + }) + + it('should allow to have a valid /not-found route', async () => { + const html = await next.render('/not-found') + expect(html).toContain("I'm still a valid page") + }) + } + + describe('with default runtime', () => { + runTests() + }) + + describe('with runtime = edge', () => { + let originalLayout = '' + + beforeAll(async () => { + await next.stop() + originalLayout = await next.readFile('app/layout.js') + await next.patchFile( + 'app/layout.js', + `export const runtime = 'edge'\n${originalLayout}` + ) + await next.start() + }) + afterAll(async () => { + await next.patchFile('app/layout.js', originalLayout) + }) + + runTests() + }) + } +) diff --git a/test/e2e/app-dir/not-found/next.config.js b/test/e2e/app-dir/not-found/next.config.js deleted file mode 100644 index 4ba52ba2c8df6..0000000000000 --- a/test/e2e/app-dir/not-found/next.config.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = {} From d19882118ec75e9a76773726a9fb5cbadf10ef4a Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 7 Aug 2023 18:55:02 +0200 Subject: [PATCH 2/2] fix lint --- test/e2e/app-dir/not-found/conflict-route/index.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/app-dir/not-found/conflict-route/index.test.ts b/test/e2e/app-dir/not-found/conflict-route/index.test.ts index ad934522a6e05..46b33d3e676f5 100644 --- a/test/e2e/app-dir/not-found/conflict-route/index.test.ts +++ b/test/e2e/app-dir/not-found/conflict-route/index.test.ts @@ -1,5 +1,4 @@ import { createNextDescribe } from 'e2e-utils' -import { check } from 'next-test-utils' createNextDescribe( 'app dir - not-found - conflict route',