Skip to content

Commit

Permalink
Make sure the global not found route doesn't conflict with existing /…
Browse files Browse the repository at this point in the history
…not-found route (#47619)

In #47328 we made the root level `/not-found.js` a special entry, to
override the page 404 during builds. However, it's possible that the
user has a valid `/not-found/page.js` route that might conflict with
this special entry. This PR changes the entry to be `/_not-found` so it
will never conflict with existing valid entries.
  • Loading branch information
shuding authored Mar 28, 2023
1 parent 28d56ec commit 54fce53
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export function createPagesMapping({
let pageKey = getPageFromPath(pagePath, pageExtensions)
if (isAppRoute) {
pageKey = pageKey.replace(/%5F/g, '_')
pageKey = pageKey.replace(/^\/not-found$/g, '/_not-found')
}

if (pageKey in result) {
Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ export default async function build(

const conflictingPublicFiles: string[] = []
const hasPages404 = mappedPages['/404']?.startsWith(PAGES_DIR_ALIAS)
const hasApp404 = !!mappedAppPages?.['/not-found']
const hasApp404 = !!mappedAppPages?.['/_not-found']
const hasCustomErrorPage =
mappedPages['/_error'].startsWith(PAGES_DIR_ALIAS)

Expand Down Expand Up @@ -2493,7 +2493,7 @@ export default async function build(

routes.forEach((route) => {
if (isDynamicRoute(page) && route === page) return
if (route === '/not-found') return
if (route === '/_not-found') return

let revalidate = exportConfig.initialPageRevalidationMap[route]

Expand Down Expand Up @@ -2703,7 +2703,7 @@ export default async function build(
distDir,
'server',
'app',
'not-found.html'
'_not-found.html'
)
const updatedRelativeDest = path
.join('pages', '404.html')
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ export default async function exportPage({
try {
curRenderOpts.params ||= {}

const isNotFoundPage = page === '/not-found'
const isNotFoundPage = page === '/_not-found'
const result = await renderToHTMLOrFlight(
req as any,
res as any,
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2247,7 +2247,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
if (this.hasAppDir) {
// Use the not-found entry in app directory
result = await this.findPageComponents({
pathname: '/not-found',
pathname: this.renderOpts.dev ? '/not-found' : '/_not-found',
query,
params: {},
isAppPath: true,
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/not-found/app/not-found/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <h1>I'm still a valid page</h1>
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/not-found/not-found.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ createNextDescribe(
expect(html).toContain('This Is The Not Found Page')
})

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")
})

if (!isNextDev) {
it('should create the 404 mapping and copy the file to pages', async () => {
const html = await next.readFile('.next/server/pages/404.html')
Expand Down

0 comments on commit 54fce53

Please sign in to comment.