Assign default not-found boundary if custom not-found is not present …

…for root layer only (#54185) Fixes #54174 We should only add default not-found boundary to loader tree components for root layout. It was accidently added for children routes before
vercel · Aug 17, 2023 · e20c8c8 · e20c8c8
1 parent 847c14e
commit e20c8c8
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 26 deletions.
diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -53,6 +53,8 @@ const GLOBAL_ERROR_FILE_TYPE = 'global-error'
 const PAGE_SEGMENT = 'page$'
 const PARALLEL_CHILDREN_SEGMENT = 'children$'
 
+const defaultNotFoundPath = 'next/dist/client/components/not-found-error'
+
 type DirResolver = (pathToResolve: string) => string
 type PathResolver = (
   pathname: string
@@ -311,6 +313,14 @@ async function createTreeCodeFromPath(
         ([, filePath]) => filePath !== undefined
       )
 
+      // Add default not found error as root not found if not present
+      const hasNotFound = definedFilePaths.some(
+        ([type]) => type === 'not-found'
+      )
+      if (isRootLayer && !hasNotFound) {
+        definedFilePaths.push(['not-found', defaultNotFoundPath])
+      }
+
       if (!rootLayout) {
         const layoutPath = definedFilePaths.find(
           ([type]) => type === 'layout'
@@ -321,17 +331,6 @@ async function createTreeCodeFromPath(
           globalError = await resolver(
             `${path.dirname(layoutPath)}/${GLOBAL_ERROR_FILE_TYPE}`
           )
-
-          const hasNotFound = definedFilePaths.some(
-            ([type]) => type === 'not-found'
-          )
-          // Add default not found error as root not found if not present
-          if (!hasNotFound) {
-            const notFoundPath = 'next/dist/client/components/not-found-error'
-            if (notFoundPath) {
-              definedFilePaths.push(['not-found', notFoundPath])
-            }
-          }
         }
       }
 
@@ -350,7 +349,7 @@ async function createTreeCodeFromPath(
       if (isNotFoundRoute && normalizedParallelKey === 'children') {
         const notFoundPath =
           definedFilePaths.find(([type]) => type === 'not-found')?.[1] ??
-          'next/dist/client/components/not-found-error'
+          defaultNotFoundPath
         subtreeCode = `{
           children: ['__PAGE__', {}, {
             page: [

diff --git a/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/[id]/page.js b/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/[id]/page.js
@@ -0,0 +1,12 @@
+import { notFound } from 'next/navigation'
+
+// avoid static generation to fill the dynamic params
+export const dynamic = 'force-dynamic'
+
+export default function Page({ params: { id } }) {
+  if (id === '404') {
+    notFound()
+  }
+
+  return <p id="page">{`dynamic-layout-without-not-found [id]`}</p>
+}
diff --git a/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/layout.js b/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/layout.js
@@ -0,0 +1,8 @@
+export default function Layout({ children }) {
+  return (
+    <div>
+      <h1>Dynamic with Layout</h1>
+      {children}
+    </div>
+  )
+}
diff --git a/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/page.js b/test/e2e/app-dir/not-found/basic/app/dynamic-layout-without-not-found/page.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>dynamic-with-layout</div>
+}
diff --git a/test/e2e/app-dir/not-found/basic/app/dynamic/[id]/page.js b/test/e2e/app-dir/not-found/basic/app/dynamic/[id]/page.js
@@ -3,8 +3,10 @@ import { notFound } from 'next/navigation'
 // avoid static generation to fill the dynamic params
 export const dynamic = 'force-dynamic'
 
-export default function Page() {
-  notFound()
+export default function Page({ params: { id } }) {
+  if (id === '404') {
+    notFound()
+  }
 
-  return <>{`dynamic [id]`}</>
+  return <p id="page">{`dynamic [id]`}</p>
 }
diff --git a/test/e2e/app-dir/not-found/basic/app/not-found.js b/test/e2e/app-dir/not-found/basic/app/not-found.js
@@ -1,7 +1,7 @@
 export default function NotFound() {
   return (
     <>
-      <h1>This Is The Not Found Page</h1>
+      <h1>Root Not Found</h1>
 
       <div id="timestamp">{Date.now()}</div>
     </>

diff --git a/test/e2e/app-dir/not-found/basic/index.test.ts b/test/e2e/app-dir/not-found/basic/index.test.ts
@@ -29,15 +29,15 @@ createNextDescribe(
       it('should use root not-found content for 404 html', async () => {
         // static /404 page will use /_not-found content
         const page404 = await next.readFile('.next/server/pages/404.html')
-        expect(page404).toContain('This Is The Not Found Page')
+        expect(page404).toContain('Root Not Found')
       })
     }
 
     const runTests = ({ isEdge }: { isEdge: boolean }) => {
       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'
+          'Root Not Found'
         )
         // should contain root layout content
         expect(await browser.elementByCss('#layout-nav').text()).toBe('Navbar')
@@ -48,11 +48,40 @@ createNextDescribe(
         const browserDynamic = await next.browser('/dynamic')
         expect(await browserDynamic.elementByCss('main').text()).toBe('dynamic')
 
-        // `/dynamic/[id]` calling notFound() will match the same level not-found boundary
-        const browserDynamicId = await next.browser('/dynamic/123')
-        expect(await browserDynamicId.elementByCss('#not-found').text()).toBe(
+        // `/dynamic/404` calling notFound() will match the same level not-found boundary
+        const browserDynamic404 = await next.browser('/dynamic/404')
+        expect(await browserDynamic404.elementByCss('#not-found').text()).toBe(
           'dynamic/[id] not found'
         )
+
+        const browserDynamicId = await next.browser('/dynamic/123')
+        expect(await browserDynamicId.elementByCss('#page').text()).toBe(
+          'dynamic [id]'
+        )
+      })
+
+      it('should escalate notFound to parent layout if no not-found boundary present in current layer', async () => {
+        const browserDynamic = await next.browser(
+          '/dynamic-layout-without-not-found'
+        )
+        expect(await browserDynamic.elementByCss('h1').text()).toBe(
+          'Dynamic with Layout'
+        )
+
+        // no not-found boundary in /dynamic-layout-without-not-found, escalate to parent layout to render root not-found
+        const browserDynamicId = await next.browser(
+          '/dynamic-layout-without-not-found/404'
+        )
+        expect(await browserDynamicId.elementByCss('h1').text()).toBe(
+          'Root Not Found'
+        )
+
+        const browserDynamic404 = await next.browser(
+          '/dynamic-layout-without-not-found/123'
+        )
+        expect(await browserDynamic404.elementByCss('#page').text()).toBe(
+          'dynamic-layout-without-not-found [id]'
+        )
       })
 
       if (isNextDev) {
@@ -74,10 +103,7 @@ createNextDescribe(
           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 check(() => browser.elementByCss('h1').text(), 'Root Not Found')
           await next.renameFile('./app/foo.js', './app/page.js')
           await check(() => browser.elementByCss('h1').text(), 'My page')
         })
@@ -86,7 +112,7 @@ createNextDescribe(
       if (!isNextDev && !isEdge) {
         it('should create the 404 mapping and copy the file to pages', async () => {
           const html = await next.readFile('.next/server/pages/404.html')
-          expect(html).toContain('This Is The Not Found Page')
+          expect(html).toContain('Root Not Found')
           expect(
             await next.readFile('.next/server/pages-manifest.json')
           ).toContain('"pages/404.html"')