Recover not found errors from flight data to render with proper bound…

…ary (#53703) ### What? We change the not-found rendering strategy to the origin one which recovers the not found error from the flight data, and hit the error boundary to display the closet not found component. For parallel `@slot` we shouldn't pass down the not-found boundary, the boundary is only for `@children`. ### Why? We're having a lot of not-found matching issues that manually searching for not found and layout won't be accurate as we have various scenarios like `(group)` routes, dynamic routes, etc. ### How? Only render html with empty body so that the error can recover from flight and render the proper not-found component during hydration. One change for metadata is that we need to get the "not-found" metadata in the initial render, so we'll catch the not-found error once there first and start render the "not-found" metadata and put it in the flight data. Then when it recovers it's still preserved. Fixes #53694
vercel · Aug 12, 2023 · 4056994 · 4056994
1 parent ea0bb97
commit 4056994
Show file tree

Hide file tree

Showing 16 changed files with 262 additions and 161 deletions.
diff --git a/...s/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx b/...s/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx
@@ -49,7 +49,8 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) {
   )
 
   // TODO: This test is flaky, so it needs a particularly long timeout.
-  it(
+  // TODO(WEB-980) Fix this test once we no longer throw an error when rendering a 404 page.
+  it.skip(
     'renders a custom 404 page',
     async () => {
       await harness.load(iframe, '/not-found')

diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -319,6 +319,17 @@ 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])
+            }
+          }
         }
       }
 

diff --git a/packages/next/src/client/components/not-found-boundary.tsx b/packages/next/src/client/components/not-found-boundary.tsx
@@ -1,3 +1,5 @@
+'use client'
+
 import React from 'react'
 import { usePathname } from './navigation'
 

diff --git a/...ages/next/src/client/components/error.tsx → ...src/client/components/not-found-error.tsx b/...ages/next/src/client/components/error.tsx → ...src/client/components/not-found-error.tsx
@@ -35,7 +35,7 @@ const styles: Record<string, React.CSSProperties> = {
   },
 }
 
-export function NotFound() {
+export default function NotFound() {
   return (
     <>
       {/* <head> */}

diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx
@@ -16,10 +16,11 @@ import {
   AppLinksMeta,
 } from './generate/opengraph'
 import { IconsMetadata } from './generate/icons'
-import { accumulateMetadata, resolveMetadata } from './resolve-metadata'
+import { resolveMetadata } from './resolve-metadata'
 import { MetaFilter } from './generate/meta'
 import { ResolvedMetadata } from './types/metadata-interface'
 import { createDefaultMetadata } from './default-metadata'
+import { isNotFoundError } from '../../client/components/not-found'
 
 // Use a promise to share the status of the metadata resolving,
 // returning two components `MetadataTree` and `MetadataOutlet`
@@ -55,24 +56,44 @@ export function createMetadataComponents({
   async function MetadataTree() {
     const defaultMetadata = createDefaultMetadata()
     let metadata: ResolvedMetadata | undefined = defaultMetadata
-    try {
-      const resolvedMetadata = await resolveMetadata({
-        tree,
-        parentParams: {},
-        metadataItems: [],
-        searchParams,
-        getDynamicParamFromSegment,
-        errorConvention: errorType === 'redirect' ? undefined : errorType,
-      })
+    let error: any
+    const errorMetadataItem: [null, null] = [null, null]
+    const errorConvention = errorType === 'redirect' ? undefined : errorType
 
-      // Skip for redirect case as for the temporary redirect case we don't need the metadata on client
-      if (errorType === 'redirect') {
-        metadata = defaultMetadata
-      } else {
-        metadata = await accumulateMetadata(resolvedMetadata, metadataContext)
-      }
+    const [resolvedMetadata, resolvedError] = await resolveMetadata({
+      tree,
+      parentParams: {},
+      metadataItems: [],
+      errorMetadataItem,
+      searchParams,
+      getDynamicParamFromSegment,
+      errorConvention,
+      metadataContext,
+    })
+    if (!resolvedError) {
+      metadata = resolvedMetadata
       resolve(undefined)
-    } catch (error: any) {
+    } else {
+      error = resolvedError
+      // If the error triggers in initial metadata resolving, re-resolve with proper error type.
+      // They'll be saved for flight data, when hydrates, it will replaces the SSR'd metadata with this.
+      // for not-found error: resolve not-found metadata
+      if (!errorType && isNotFoundError(resolvedError)) {
+        const [notFoundMetadata, notFoundMetadataError] = await resolveMetadata(
+          {
+            tree,
+            parentParams: {},
+            metadataItems: [],
+            errorMetadataItem,
+            searchParams,
+            getDynamicParamFromSegment,
+            errorConvention: 'not-found',
+            metadataContext,
+          }
+        )
+        metadata = notFoundMetadata
+        error = notFoundMetadataError || error
+      }
       resolve(error)
     }
 

diff --git a/packages/next/src/lib/metadata/resolve-metadata.ts b/packages/next/src/lib/metadata/resolve-metadata.ts
@@ -15,7 +15,7 @@ import { resolveTitle } from './resolvers/resolve-title'
 import { resolveAsArrayOrUndefined } from './generate/utils'
 import { isClientReference } from '../client-reference'
 import {
-  getErrorOrLayoutModule,
+  getComponentTypeModule,
   getLayoutOrPageModule,
   LoaderTree,
 } from '../../server/lib/app-dir-module'
@@ -315,21 +315,26 @@ async function resolveStaticMetadata(components: ComponentsType, props: any) {
 // [layout.metadata, static files metadata] -> ... -> [page.metadata, static files metadata]
 export async function collectMetadata({
   tree,
-  metadataItems: array,
+  metadataItems,
+  errorMetadataItem,
   props,
   route,
   errorConvention,
 }: {
   tree: LoaderTree
   metadataItems: MetadataItems
+  errorMetadataItem: MetadataItems[number]
   props: any
   route: string
   errorConvention?: 'not-found'
 }) {
   let mod
   let modType
+  const hasErrorConventionComponent = Boolean(
+    errorConvention && tree[2][errorConvention]
+  )
   if (errorConvention) {
-    mod = await getErrorOrLayoutModule(tree, errorConvention)
+    mod = await getComponentTypeModule(tree, 'layout')
     modType = errorConvention
   } else {
     ;[mod, modType] = await getLayoutOrPageModule(tree)
@@ -344,13 +349,23 @@ export async function collectMetadata({
     ? await getDefinedMetadata(mod, props, { route })
     : null
 
-  array.push([metadataExport, staticFilesMetadata])
+  metadataItems.push([metadataExport, staticFilesMetadata])
+
+  if (hasErrorConventionComponent && errorConvention) {
+    const errorMod = await getComponentTypeModule(tree, errorConvention)
+    const errorMetadataExport = errorMod
+      ? await getDefinedMetadata(errorMod, props, { route })
+      : null
+    errorMetadataItem[0] = errorMetadataExport
+    errorMetadataItem[1] = staticFilesMetadata
+  }
 }
 
-export async function resolveMetadata({
+export async function resolveMetadataItems({
   tree,
   parentParams,
   metadataItems,
+  errorMetadataItem,
   treePrefix = [],
   getDynamicParamFromSegment,
   searchParams,
@@ -359,6 +374,7 @@ export async function resolveMetadata({
   tree: LoaderTree
   parentParams: { [key: string]: any }
   metadataItems: MetadataItems
+  errorMetadataItem: MetadataItems[number]
   /** Provided tree can be nested subtree, this argument says what is the path of such subtree */
   treePrefix?: string[]
   getDynamicParamFromSegment: GetDynamicParamFromSegment
@@ -392,6 +408,7 @@ export async function resolveMetadata({
   await collectMetadata({
     tree,
     metadataItems,
+    errorMetadataItem,
     errorConvention,
     props: layerProps,
     route: currentTreePrefix
@@ -402,9 +419,10 @@ export async function resolveMetadata({
 
   for (const key in parallelRoutes) {
     const childTree = parallelRoutes[key]
-    await resolveMetadata({
+    await resolveMetadataItems({
       tree: childTree,
       metadataItems,
+      errorMetadataItem,
       parentParams: currentParams,
       treePrefix: currentTreePrefix,
       searchParams,
@@ -413,6 +431,12 @@ export async function resolveMetadata({
     })
   }
 
+  if (Object.keys(parallelRoutes).length === 0 && errorConvention) {
+    // If there are no parallel routes, place error metadata as the last item.
+    // e.g. layout -> layout -> not-found
+    metadataItems.push(errorMetadataItem)
+  }
+
   return metadataItems
 }
 
@@ -543,3 +567,43 @@ export async function accumulateMetadata(
 
   return postProcessMetadata(resolvedMetadata, titleTemplates)
 }
+
+export async function resolveMetadata({
+  tree,
+  parentParams,
+  metadataItems,
+  errorMetadataItem,
+  getDynamicParamFromSegment,
+  searchParams,
+  errorConvention,
+  metadataContext,
+}: {
+  tree: LoaderTree
+  parentParams: { [key: string]: any }
+  metadataItems: MetadataItems
+  errorMetadataItem: MetadataItems[number]
+  /** Provided tree can be nested subtree, this argument says what is the path of such subtree */
+  treePrefix?: string[]
+  getDynamicParamFromSegment: GetDynamicParamFromSegment
+  searchParams: { [key: string]: any }
+  errorConvention: 'not-found' | undefined
+  metadataContext: MetadataContext
+}): Promise<[ResolvedMetadata, any]> {
+  const resolvedMetadataItems = await resolveMetadataItems({
+    tree,
+    parentParams,
+    metadataItems,
+    errorMetadataItem,
+    getDynamicParamFromSegment,
+    searchParams,
+    errorConvention,
+  })
+  let metadata: ResolvedMetadata = createDefaultMetadata()
+  let error
+  try {
+    metadata = await accumulateMetadata(resolvedMetadataItems, metadataContext)
+  } catch (err: any) {
+    error = err
+  }
+  return [metadata, error]
+}