From d3a9da4878aaa4b610c45114814c8d8849fdf28a Mon Sep 17 00:00:00 2001
From: Jiachi Liu
Date: Sat, 2 Sep 2023 22:56:39 +0200
Subject: [PATCH] Fix group routes custom root not-found
---
.../build/webpack/loaders/next-app-loader.ts | 12 ++++++--
.../app-dir/not-found-default/index.test.ts | 1 -
.../app/(group)/group-dynamic/[id]/page.js | 11 +++++++
.../group-route-root-not-found/app/layout.js | 11 +++++++
.../app/not-found.js | 3 ++
.../group-route-root-not-found/app/page.js | 3 ++
.../group-route-root-not-found/index.test.ts | 29 +++++++++++++++++++
7 files changed, 66 insertions(+), 4 deletions(-)
create mode 100644 test/e2e/app-dir/not-found/group-route-root-not-found/app/(group)/group-dynamic/[id]/page.js
create mode 100644 test/e2e/app-dir/not-found/group-route-root-not-found/app/layout.js
create mode 100644 test/e2e/app-dir/not-found/group-route-root-not-found/app/not-found.js
create mode 100644 test/e2e/app-dir/not-found/group-route-root-not-found/app/page.js
create mode 100644 test/e2e/app-dir/not-found/group-route-root-not-found/index.test.ts
diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
index 841ca3fd1500b..551ac42bb045c 100644
--- a/packages/next/src/build/webpack/loaders/next-app-loader.ts
+++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -182,6 +182,9 @@ async function createTreeCodeFromPath(
const isNotFoundRoute = page === '/_not-found'
const isDefaultNotFound = isAppBuiltinNotFoundPage(pagePath)
const appDirPrefix = isDefaultNotFound ? APP_DIR_ALIAS : splittedPath[0]
+ const hasRootNotFound = await resolver(
+ `${appDirPrefix}/${FILE_TYPES['not-found']}`
+ )
const pages: string[] = []
let rootLayout: string | undefined
@@ -321,15 +324,18 @@ async function createTreeCodeFromPath(
)
// Add default not found error as root not found if not present
- const hasRootNotFound = definedFilePaths.some(
+ const hasNotFoundFile = definedFilePaths.some(
([type]) => type === 'not-found'
)
// If the first layer is a group route, we treat it as root layer
const isFirstLayerGroupRoute =
segments.length === 1 &&
subSegmentPath.filter((seg) => isGroupSegment(seg)).length === 1
- if ((isRootLayer || isFirstLayerGroupRoute) && !hasRootNotFound) {
- definedFilePaths.push(['not-found', defaultNotFoundPath])
+ if ((isRootLayer || isFirstLayerGroupRoute) && !hasNotFoundFile) {
+ // If you already have a root not found, don't insert default not-found to group routes root
+ if (!(hasRootNotFound && isFirstLayerGroupRoute)) {
+ definedFilePaths.push(['not-found', defaultNotFoundPath])
+ }
}
if (!rootLayout) {
diff --git a/test/e2e/app-dir/not-found-default/index.test.ts b/test/e2e/app-dir/not-found-default/index.test.ts
index 1a8bb252cecce..06037098b3751 100644
--- a/test/e2e/app-dir/not-found-default/index.test.ts
+++ b/test/e2e/app-dir/not-found-default/index.test.ts
@@ -96,7 +96,6 @@ createNextDescribe(
await browser.loadPage(next.url + '/group-dynamic/404')
expect(await browser.elementByCss('.next-error-h1').text()).toBe('404')
- // Using default layout
expect(await browser.elementByCss('html').getAttribute('class')).toBe(
'group-root-layout'
)
diff --git a/test/e2e/app-dir/not-found/group-route-root-not-found/app/(group)/group-dynamic/[id]/page.js b/test/e2e/app-dir/not-found/group-route-root-not-found/app/(group)/group-dynamic/[id]/page.js
new file mode 100644
index 0000000000000..0f514527eb10c
--- /dev/null
+++ b/test/e2e/app-dir/not-found/group-route-root-not-found/app/(group)/group-dynamic/[id]/page.js
@@ -0,0 +1,11 @@
+import { notFound } from 'next/navigation'
+
+export const dynamic = 'force-dynamic'
+
+export default function Page({ params }) {
+ if (params.id === '404') {
+ notFound()
+ }
+
+ return {`group-dynamic [id]`}
+}
diff --git a/test/e2e/app-dir/not-found/group-route-root-not-found/app/layout.js b/test/e2e/app-dir/not-found/group-route-root-not-found/app/layout.js
new file mode 100644
index 0000000000000..7e3891a0b71ed
--- /dev/null
+++ b/test/e2e/app-dir/not-found/group-route-root-not-found/app/layout.js
@@ -0,0 +1,11 @@
+export default function RootLayout({ children }) {
+ return (
+
+
+
+ Root layout
+ {children}
+
+
+ )
+}
diff --git a/test/e2e/app-dir/not-found/group-route-root-not-found/app/not-found.js b/test/e2e/app-dir/not-found/group-route-root-not-found/app/not-found.js
new file mode 100644
index 0000000000000..410d06236ed58
--- /dev/null
+++ b/test/e2e/app-dir/not-found/group-route-root-not-found/app/not-found.js
@@ -0,0 +1,3 @@
+export default function NotFound() {
+ return Not found placeholder
+}
diff --git a/test/e2e/app-dir/not-found/group-route-root-not-found/app/page.js b/test/e2e/app-dir/not-found/group-route-root-not-found/app/page.js
new file mode 100644
index 0000000000000..c303be8817213
--- /dev/null
+++ b/test/e2e/app-dir/not-found/group-route-root-not-found/app/page.js
@@ -0,0 +1,3 @@
+export default function Home() {
+ return Home
+}
diff --git a/test/e2e/app-dir/not-found/group-route-root-not-found/index.test.ts b/test/e2e/app-dir/not-found/group-route-root-not-found/index.test.ts
new file mode 100644
index 0000000000000..74b32e5bd2ece
--- /dev/null
+++ b/test/e2e/app-dir/not-found/group-route-root-not-found/index.test.ts
@@ -0,0 +1,29 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+ 'app dir - group routes with root not-found',
+ {
+ files: __dirname,
+ skipDeployment: true,
+ },
+ ({ next }) => {
+ it('should render default 404 with root layout for non-existent page', async () => {
+ const browser = await next.browser('/non-existent')
+ expect(await browser.elementByCss('p').text()).toBe(
+ 'Not found placeholder'
+ )
+ expect(await browser.elementByCss('h1').text()).toBe('Root layout')
+ })
+
+ it('should render root not found for group routes if hit 404', async () => {
+ const browser = await next.browser('/group-dynamic/123')
+ expect(await browser.elementByCss('p').text()).toBe('group-dynamic [id]')
+
+ await browser.loadPage(next.url + '/group-dynamic/404')
+ expect(await browser.elementByCss('p').text()).toBe(
+ 'Not found placeholder'
+ )
+ expect(await browser.elementByCss('h1').text()).toBe('Root layout')
+ })
+ }
+)