Fix not-found rendering in production with edge (#53687)

Edge runtime doesn't have `/404` entry points as not-found, if server hits 404 it's hitting `/_not-found` entry point of edge bundles unlike nodejs server rendering.

In app-render it needs the information that when `pathname` (or can call `pagePath`) is `/404` it can render the `not-found.js` components properly

Separate the not-found test suite into 2 parts, a basic case testing all urls with `not-found.js` boundary, and another conflict route case with `not-found.js` with `not-found/` route at the same time

Fixes #53652
Fixes #53210
This commit is contained in:
Jiachi Liu 2023-08-07 22:05:22 +02:00 committed by GitHub
parent 033732a3e5
commit 9035f14dda
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 106 additions and 24 deletions

View file

@ -1411,7 +1411,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
{ req, res, pathname, renderOpts: opts }: RequestContext,
{ components, query }: FindComponentsResult
): Promise<ResponsePayload | null> {
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<ServerOptions extends Options = Options> {
(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

View file

@ -330,6 +330,11 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
)
}
// 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,

View file

@ -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) {

View file

@ -0,0 +1,16 @@
export default function Layout({ children }) {
return (
<html>
<head />
<body>
<header>
<nav id="layout-nav">Navbar</nav>
</header>
<main>{children}</main>
<footer>
<p id="layout-footer">Footer</p>
</footer>
</body>
</html>
)
}

View file

@ -0,0 +1,11 @@
export default function NotFound() {
return (
<>
<h1>This Is The Not Found Page</h1>
<div id="timestamp">{Date.now()}</div>
</>
)
}
NotFound.displayName = 'NotFound'

View file

@ -0,0 +1,3 @@
export default function Page() {
return <h1>My page</h1>
}

View file

@ -0,0 +1,49 @@
import { createNextDescribe } from 'e2e-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()
})
}
)

View file

@ -1 +0,0 @@
module.exports = {}