Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover not found errors from flight data to render with proper boundary #53703

Merged
merged 25 commits into from
Aug 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with @alexkirsz that not-found is not handled well in turbopack so we add a ticket to track later

it.skip(
'renders a custom 404 page',
async () => {
await harness.load(iframe, '/not-found')
Expand Down
11 changes: 11 additions & 0 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/client/components/not-found-boundary.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client'

import React from 'react'
import { usePathname } from './navigation'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const styles: Record<string, React.CSSProperties> = {
},
}

export function NotFound() {
export default function NotFound() {
return (
<>
{/* <head> */}
Expand Down
55 changes: 38 additions & 17 deletions packages/next/src/lib/metadata/metadata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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)
}

Expand Down
76 changes: 70 additions & 6 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -392,6 +408,7 @@ export async function resolveMetadata({
await collectMetadata({
tree,
metadataItems,
errorMetadataItem,
errorConvention,
props: layerProps,
route: currentTreePrefix
Expand All @@ -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,
Expand All @@ -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
}

Expand Down Expand Up @@ -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]
}
Loading