Add default not found to loader tree of group routes root layer (#54228)

huozhi · web-flow · commit c305bf6afbb0 · 2023-08-18T16:56:55.000Z
For group routes, unlike normal routes, the root layout is located in the "group"'s segment instead of root layer. To make it also able to leverage the default not found error component as root not found component, we need to determine if we're in the root group segment in the loader tree, and add the not-found boundary for it. If you compre the loader tree visually they will look like this: Normal route tree structure ``` ['', { children: [segment, {...}, {...}]}, { layout: ..., 'not-found': ... } ] ``` Group routes ``` ['' { children: ['(group)', {...}, { layout, 'not-found': ... }]} {} ] ``` Comparing to normal case, we go 1 layer down in the tree for group routes, then insert the not-found boundary there Fixes #52255 Closes NEXT-1532
diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -75,6 +75,10 @@ export type ComponentsType = {
   readonly defaultPage?: ModuleReference
 }
 
+function isGroupSegment(segment: string) {
+  return segment.startsWith('(') && segment.endsWith(')')
+}
+
 async function createAppRouteCode({
   name,
   page,
@@ -225,6 +229,7 @@ async function createTreeCodeFromPath(
 
     // Existing tree are the children of the current segment
     const props: Record<string, string> = {}
+    // Root layer could be 1st layer of normal routes
     const isRootLayer = segments.length === 0
     const isRootLayoutOrRootPage = segments.length <= 1
 
@@ -314,10 +319,14 @@ async function createTreeCodeFromPath(
       )
 
       // Add default not found error as root not found if not present
-      const hasNotFound = definedFilePaths.some(
+      const hasRootNotFound = definedFilePaths.some(
         ([type]) => type === 'not-found'
       )
-      if (isRootLayer && !hasNotFound) {
+      // 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])
       }
 
diff --git a/test/e2e/app-dir/not-found-default/app/(group)/group-dynamic/[id]/page.js b/test/e2e/app-dir/not-found-default/app/(group)/group-dynamic/[id]/page.js
@@ -0,0 +1,9 @@
+import { notFound } from 'next/navigation'
+
+export default function Page({ params }) {
+  if (params.id === '404') {
+    notFound()
+  }
+
+  return <p id="page">{`group-dynamic [id]`}</p>
+}
diff --git a/test/e2e/app-dir/not-found-default/app/(group)/layout.js b/test/e2e/app-dir/not-found-default/app/(group)/layout.js
@@ -0,0 +1,7 @@
+export default function RootLayout({ children }) {
+  return (
+    <html className="group-root-layout">
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/not-found-default/index.test.ts b/test/e2e/app-dir/not-found-default/index.test.ts
@@ -72,5 +72,18 @@ createNextDescribe(
         'This page could not be found.'
       )
     })
+
+    it('should render default not found for group routes if not found is not defined', async () => {
+      const browser = await next.browser('/group-dynamic/123')
+      expect(await browser.elementByCss('#page').text()).toBe(
+        'group-dynamic [id]'
+      )
+
+      await browser.loadPage(next.url + '/group-dynamic/404')
+      expect(await browser.elementByCss('.next-error-h1').text()).toBe('404')
+      expect(await browser.elementByCss('html').getAttribute('class')).toBe(
+        'group-root-layout'
+      )
+    })
   }
 )

Original file line number	Diff line number	Diff line change
`@@ -72,5 +72,18 @@ createNextDescribe(`
`72`	`72`	`'This page could not be found.'`
`73`	`73`	`)`
`74`	`74`	`})`
	`75`	`+`
	`76`	`+ it('should render default not found for group routes if not found is not defined', async () => {`
	`77`	`+ const browser = await next.browser('/group-dynamic/123')`
	`78`	`+ expect(await browser.elementByCss('#page').text()).toBe(`
	`79`	`+ 'group-dynamic [id]'`
	`80`	`+ )`
	`81`	`+`
	`82`	`+ await browser.loadPage(next.url + '/group-dynamic/404')`
	`83`	`+ expect(await browser.elementByCss('.next-error-h1').text()).toBe('404')`
	`84`	`+ expect(await browser.elementByCss('html').getAttribute('class')).toBe(`
	`85`	`+ 'group-root-layout'`
	`86`	`+ )`
	`87`	`+ })`
`75`	`88`	`}`
`76`	`89`	`)`