Default app router not found (#54199)

Adding a default app router not-found error page in production. We introduced a custom global not-found page if you have `not-found.js` file under app directory. Next.js will still routed you to default pages 404 if there's no routes matched when you don't have a custom `not-found.js`. This PR creates a default layout (only having `html` and `body`) and will use the default not-found component as children to build a default 404 page for app router, and server will route to that page when there's any app router pages existed. This will also fix the hmr issue that when you hit a pathname doesn't exist, nextjs will compile `/_error` for you which is not expected, now we're using `/not-found` instead Closes NEXT-1359
vercel · Aug 28, 2023 · 8ed492f · 8ed492f
1 parent a6a452b
commit 8ed492f
Show file tree

Hide file tree

Showing 18 changed files with 159 additions and 43 deletions.
diff --git a/packages/next-swc/crates/next-core/src/app_structure.rs b/packages/next-swc/crates/next-core/src/app_structure.rs
@@ -754,7 +754,7 @@ async fn directory_tree_to_entrypoints_internal(
         // Next.js has this logic in "collect-app-paths", where the root not-found page
         // is considered as its own entry point.
         if let Some(_not_found) = components.not_found {
-            let tree = LoaderTree {
+            let dev_not_found_tree = LoaderTree {
                 segment: directory_name.to_string(),
                 parallel_routes: indexmap! {
                     "children".to_string() => LoaderTree {
@@ -771,23 +771,53 @@ async fn directory_tree_to_entrypoints_internal(
                 components: components.without_leafs().cell(),
             }
             .cell();
+
             add_app_page(
                 app_dir,
                 &mut result,
                 "/not-found".to_string(),
                 "/not-found".to_string(),
-                tree,
+                dev_not_found_tree,
             )
             .await?;
             add_app_page(
                 app_dir,
                 &mut result,
                 "/_not-found".to_string(),
                 "/_not-found".to_string(),
-                tree,
+                dev_not_found_tree,
             )
             .await?;
         }
+    } else {
+        // Create default not-found page for production if there's no customized
+        // not-found
+        let prod_not_found_tree = LoaderTree {
+            segment: directory_name.to_string(),
+            parallel_routes: indexmap! {
+                "children".to_string() => LoaderTree {
+                    segment: "__PAGE__".to_string(),
+                    parallel_routes: IndexMap::new(),
+                    components: Components {
+                        page: Some(get_next_package(app_dir).join("dist/client/components/not-found-error.js".to_string())),
+                        ..Default::default()
+                    }
+                    .cell(),
+                }
+                .cell(),
+            },
+            components: components.without_leafs().cell(),
+        }
+        .cell();
+
+        add_app_page(
+            app_dir,
+            &mut result,
+            "/_not-found".to_string(),
+            "/_not-found".to_string(),
+            prod_not_found_tree,
+        )
+        .await?;
     }
 
     for (subdir_name, &subdirectory) in subdirectories.iter() {

diff --git a/packages/next/src/build/entries.ts b/packages/next/src/build/entries.ts
@@ -264,7 +264,19 @@ export function createPagesMapping({
     {}
   )
 
-  if (pagesType !== 'pages') {
+  if (pagesType === 'app') {
+    const hasAppPages = Object.keys(pages).some((page) =>
+      page.endsWith('/page')
+    )
+    return {
+      // If there's any app pages existed, add a default not-found page.
+      // If there's any custom not-found page existed, it will override the default one.
+      ...(hasAppPages && {
+        '/_not-found': 'next/dist/client/components/not-found-error',
+      }),
+      ...pages,
+    }
+  } else if (pagesType === 'root') {
     return pages
   }
 

diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -109,6 +109,7 @@ import {
   copyTracedFiles,
   isReservedPage,
   AppConfig,
+  isAppBuiltinNotFoundPage,
 } from './utils'
 import { writeBuildId } from './write-build-id'
 import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
@@ -1495,12 +1496,18 @@ export default async function build(
                   }
                 }
 
+                const pageFilePath = isAppBuiltinNotFoundPage(pagePath)
+                  ? require.resolve(
+                      'next/dist/client/components/not-found-error'
+                    )
+                  : path.join(
+                      (pageType === 'pages' ? pagesDir : appDir) || '',
+                      pagePath
+                    )
+
                 const staticInfo = pagePath
                   ? await getPageStaticInfo({
-                      pageFilePath: path.join(
-                        (pageType === 'pages' ? pagesDir : appDir) || '',
-                        pagePath
-                      ),
+                      pageFilePath,
                       nextConfig: config,
                       pageType,
                     })

diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts
@@ -1999,6 +1999,12 @@ export function isReservedPage(page: string) {
   return RESERVED_PAGE.test(page)
 }
 
+export function isAppBuiltinNotFoundPage(page: string) {
+  return /next[\\/]dist[\\/]client[\\/]components[\\/]not-found-error/.test(
+    page
+  )
+}
+
 export function isCustomErrorPage(page: string) {
   return page === '/404' || page === '/500'
 }

diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -21,6 +21,7 @@ import { AppPathnameNormalizer } from '../../../server/future/normalizers/built/
 import { AppBundlePathNormalizer } from '../../../server/future/normalizers/built/app/app-bundle-path-normalizer'
 import { MiddlewareConfig } from '../../analysis/get-page-static-info'
 import { getFilenameAndExtension } from './next-metadata-route-loader'
+import { isAppBuiltinNotFoundPage } from '../../utils'
 import { loadEntrypoint } from '../../load-entrypoint'
 
 export type AppLoaderOptions = {
@@ -181,9 +182,10 @@ async function createTreeCodeFromPath(
   globalError: string | undefined
 }> {
   const splittedPath = pagePath.split(/[\\/]/)
-  const appDirPrefix = splittedPath[0]
-  const pages: string[] = []
   const isNotFoundRoute = page === '/_not-found'
+  const isDefaultNotFound = isAppBuiltinNotFoundPage(pagePath)
+  const appDirPrefix = isDefaultNotFound ? APP_DIR_ALIAS : splittedPath[0]
+  const pages: string[] = []
 
   let rootLayout: string | undefined
   let globalError: string | undefined
@@ -244,7 +246,10 @@ async function createTreeCodeFromPath(
     let metadata: Awaited<ReturnType<typeof createStaticMetadataFromRoute>> =
       null
     const routerDirPath = `${appDirPrefix}${segmentPath}`
-    const resolvedRouteDir = await resolveDir(routerDirPath)
+    // For default not-found, don't traverse the directory to find metadata.
+    const resolvedRouteDir = isDefaultNotFound
+      ? ''
+      : await resolveDir(routerDirPath)
 
     if (resolvedRouteDir) {
       metadata = await createStaticMetadataFromRoute(resolvedRouteDir, {
@@ -336,6 +341,11 @@ async function createTreeCodeFromPath(
         )?.[1]
         rootLayout = layoutPath
 
+        if (isDefaultNotFound && !layoutPath) {
+          rootLayout = 'next/dist/client/components/default-layout'
+          definedFilePaths.push(['layout', rootLayout])
+        }
+
         if (layoutPath) {
           globalError = await resolver(
             `${path.dirname(layoutPath)}/${GLOBAL_ERROR_FILE_TYPE}`

diff --git a/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts
@@ -367,8 +367,10 @@ export class ClientReferenceManifestPlugin {
       }
 
       // Special case for the root not-found page.
-      if (/^app\/not-found(\.[^.]+)?$/.test(entryName)) {
-        manifestEntryFiles.push('app/not-found')
+      // dev: app/not-found
+      // prod: app/_not-found
+      if (/^app\/_?not-found(\.[^.]+)?$/.test(entryName)) {
+        manifestEntryFiles.push(this.dev ? 'app/not-found' : 'app/_not-found')
       }
 
       const groupName = entryNameToGroupName(entryName)

diff --git a/packages/next/src/client/components/default-layout.tsx b/packages/next/src/client/components/default-layout.tsx
@@ -0,0 +1,13 @@
+import React from 'react'
+
+export default function DefaultLayout({
+  children,
+}: {
+  children: React.ReactNode
+}) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/packages/next/src/lib/verifyRootLayout.ts b/packages/next/src/lib/verifyRootLayout.ts
@@ -41,7 +41,7 @@ export default function RootLayout({
   title: 'Next.js',
   description: 'Generated by Next.js',
 }
- 
+
 export default function RootLayout({ children }) {
  return (
     <html lang="en">
@@ -71,35 +71,42 @@ export async function verifyRootLayout({
       appDir,
       `**/layout.{${pageExtensions.join(',')}}`
     )
+    const isFileUnderAppDir = pagePath.startsWith(`${APP_DIR_ALIAS}/`)
     const normalizedPagePath = pagePath.replace(`${APP_DIR_ALIAS}/`, '')
     const pagePathSegments = normalizedPagePath.split('/')
 
     // Find an available dir to place the layout file in, the layout file can't affect any other layout.
     // Place the layout as close to app/ as possible.
     let availableDir: string | undefined
 
-    if (layoutFiles.length === 0) {
-      // If there's no other layout file we can place the layout file in the app dir.
-      // However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js)
-      // prefer creating the root layout in that route group.
-      const firstSegmentValue = pagePathSegments[0]
-      availableDir = firstSegmentValue.startsWith('(') ? firstSegmentValue : ''
-    } else {
-      pagePathSegments.pop() // remove the page from segments
+    if (isFileUnderAppDir) {
+      if (layoutFiles.length === 0) {
+        // If there's no other layout file we can place the layout file in the app dir.
+        // However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js)
+        // prefer creating the root layout in that route group.
+        const firstSegmentValue = pagePathSegments[0]
+        availableDir = firstSegmentValue.startsWith('(')
+          ? firstSegmentValue
+          : ''
+      } else {
+        pagePathSegments.pop() // remove the page from segments
 
-      let currentSegments: string[] = []
-      for (const segment of pagePathSegments) {
-        currentSegments.push(segment)
-        // Find the dir closest to app/ where a layout can be created without affecting other layouts.
-        if (
-          !layoutFiles.some((file) =>
-            file.startsWith(currentSegments.join('/'))
-          )
-        ) {
-          availableDir = currentSegments.join('/')
-          break
+        let currentSegments: string[] = []
+        for (const segment of pagePathSegments) {
+          currentSegments.push(segment)
+          // Find the dir closest to app/ where a layout can be created without affecting other layouts.
+          if (
+            !layoutFiles.some((file) =>
+              file.startsWith(currentSegments.join('/'))
+            )
+          ) {
+            availableDir = currentSegments.join('/')
+            break
+          }
         }
       }
+    } else {
+      availableDir = ''
     }
 
     if (typeof availableDir === 'string') {

diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -28,7 +28,6 @@ import {
   streamToBufferedResult,
   cloneTransformStream,
 } from '../stream-utils/node-web-streams-helper'
-import DefaultNotFound from '../../client/components/not-found-error'
 import {
   canSegmentBeOverridden,
   matchSegment,
@@ -735,7 +734,7 @@ export async function renderToHTMLOrFlight(
         const NotFoundBoundary =
           ComponentMod.NotFoundBoundary as typeof import('../../client/components/not-found-boundary').NotFoundBoundary
         Component = (componentProps: any) => {
-          const NotFoundComponent = NotFound || DefaultNotFound
+          const NotFoundComponent = NotFound
           const RootLayoutComponent = LayoutOrPage
           return (
             <NotFoundBoundary

diff --git a/packages/next/src/server/dev/on-demand-entry-handler.ts b/packages/next/src/server/dev/on-demand-entry-handler.ts
@@ -456,6 +456,16 @@ async function findPagePathData(
     }
   }
 
+  if (page === '/not-found' && appDir) {
+    return {
+      absolutePagePath: require.resolve(
+        'next/dist/client/components/not-found-error'
+      ),
+      bundlePath: 'app/not-found',
+      page: '/not-found',
+    }
+  }
+
   if (page === '/_error') {
     return {
       absolutePagePath: require.resolve('next/dist/pages/_error'),

diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts
@@ -688,7 +688,7 @@ export async function initialize(opts: {
           parsedUrl,
           'app',
           handleIndex,
-          '/_not-found',
+          opts.dev ? '/not-found' : '/_not-found',
           {
             'x-invoke-status': '404',
           }

diff --git a/packages/next/src/server/lib/router-utils/setup-dev.ts b/packages/next/src/server/lib/router-utils/setup-dev.ts
@@ -1065,9 +1065,9 @@ async function startWatcher(opts: SetupOpts) {
 
         if (isAppPath) {
           const isRootNotFound = validFileMatcher.isRootNotFound(fileName)
+          hasRootAppNotFound = true
 
           if (isRootNotFound) {
-            hasRootAppNotFound = true
             continue
           }
           if (!isRootNotFound && !validFileMatcher.isAppRouterPage(fileName)) {

diff --git a/test/development/basic/next-rs-api.test.ts b/test/development/basic/next-rs-api.test.ts
@@ -182,6 +182,7 @@ describe('next.rs api', () => {
     expect(entrypoints.done).toBe(false)
     expect(Array.from(entrypoints.value.routes.keys()).sort()).toEqual([
       '/',
+      '/_not-found',
       '/api/edge',
       '/api/nodejs',
       '/app',

diff --git a/test/e2e/app-dir/app-static/app-static.test.ts b/test/e2e/app-dir/app-static/app-static.test.ts
@@ -485,6 +485,10 @@ createNextDescribe(
 
         expect(files.sort()).toEqual(
           [
+            '_not-found.html',
+            '_not-found.js',
+            '_not-found.rsc',
+            '_not-found_client-reference-manifest.js',
             'page.js',
             'index.rsc',
             'index.html',

diff --git a/test/e2e/app-dir/create-root-layout/create-root-layout.test.ts b/test/e2e/app-dir/create-root-layout/create-root-layout.test.ts
@@ -54,7 +54,7 @@ describe('app-dir create root layout', () => {
               title: 'Next.js',
               description: 'Generated by Next.js',
             }
-             
+
             export default function RootLayout({ children }) {
              return (
                 <html lang=\\"en\\">
@@ -106,7 +106,7 @@ describe('app-dir create root layout', () => {
               title: 'Next.js',
               description: 'Generated by Next.js',
             }
-             
+
             export default function RootLayout({ children }) {
              return (
                 <html lang=\\"en\\">
@@ -158,7 +158,7 @@ describe('app-dir create root layout', () => {
               title: 'Next.js',
               description: 'Generated by Next.js',
             }
-             
+
             export default function RootLayout({ children }) {
              return (
                 <html lang=\\"en\\">

diff --git a/test/e2e/app-dir/not-found-default/app/layout.js b/test/e2e/app-dir/not-found-default/app/layout.js
@@ -5,7 +5,6 @@ import { notFound } from 'next/navigation'
 import NotFoundTrigger from './not-found-trigger'
 
 export default function Root({ children }) {
-  // notFound()
   const [clicked, setClicked] = useState(false)
   if (clicked) {
     notFound()