From a28ae633e42fd19b8c902c02d2b7964dd968f292 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sat, 15 Jul 2023 00:33:47 +0200 Subject: [PATCH] Support metadata exports for server components not-found (#52678) ### What? Support metadata exports for `not-found.js` conventions ### Why? We want to define metadata such as title or description basic properties for error pages, including 404 and 500 which referrs to `error.js` and `not-found.js` convention. See more requests in #45620 Did some research around metadata support for not-found and error convention. It's possible to support in `not-found.js` when it's server components as currently metadata is only available but for `error.js` it has to be client components for now so it's hard to support it for now as it's a boundary. ### How? We determine the convention if we're going to render is page or `not-found` boundary then we traverse the loader tree based on the convention type. One special case is for redirection the temporary metadata is not generated yet, we leave it as default now. Fixes #52636 --- .../next/src/client/on-recoverable-error.ts | 1 + packages/next/src/lib/metadata/metadata.tsx | 16 ++- .../next/src/lib/metadata/resolve-metadata.ts | 51 +++++--- .../next/src/server/app-render/app-render.tsx | 46 ++++--- .../next/src/server/lib/app-dir-module.ts | 14 +++ .../app/async/not-found/not-found.tsx | 8 ++ test/e2e/app-dir/metadata/app/layout.tsx | 1 + test/e2e/app-dir/metadata/app/not-found.tsx | 5 + test/e2e/app-dir/metadata/metadata.test.ts | 115 ++++++++++-------- 9 files changed, 171 insertions(+), 86 deletions(-) create mode 100644 test/e2e/app-dir/metadata/app/async/not-found/not-found.tsx diff --git a/packages/next/src/client/on-recoverable-error.ts b/packages/next/src/client/on-recoverable-error.ts index a58f52ce29dfb..f00a19073bca5 100644 --- a/packages/next/src/client/on-recoverable-error.ts +++ b/packages/next/src/client/on-recoverable-error.ts @@ -14,5 +14,6 @@ export default function onRecoverableError(err: any) { // Skip certain custom errors which are not expected to be reported on client if (err.digest === NEXT_DYNAMIC_NO_SSR_CODE) return + defaultOnRecoverableError(err) } diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx index ec5c5f0935162..eadd79d876b9b 100644 --- a/packages/next/src/lib/metadata/metadata.tsx +++ b/packages/next/src/lib/metadata/metadata.tsx @@ -18,6 +18,8 @@ import { import { IconsMetadata } from './generate/icons' import { accumulateMetadata, resolveMetadata } from './resolve-metadata' import { MetaFilter } from './generate/meta' +import { ResolvedMetadata } from './types/metadata-interface' +import { createDefaultMetadata } from './default-metadata' // Generate the actual React elements from the resolved metadata. export async function MetadataTree({ @@ -26,24 +28,36 @@ export async function MetadataTree({ searchParams, getDynamicParamFromSegment, appUsingSizeAdjust, + errorType, }: { tree: LoaderTree pathname: string searchParams: { [key: string]: any } getDynamicParamFromSegment: GetDynamicParamFromSegment appUsingSizeAdjust: boolean + errorType?: 'not-found' | 'redirect' }) { const metadataContext = { pathname, } + const resolvedMetadata = await resolveMetadata({ tree, parentParams: {}, metadataItems: [], searchParams, getDynamicParamFromSegment, + errorConvention: errorType === 'redirect' ? undefined : errorType, }) - const metadata = await accumulateMetadata(resolvedMetadata, metadataContext) + let metadata: ResolvedMetadata | undefined = undefined + + const defaultMetadata = createDefaultMetadata() + // 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 elements = MetaFilter([ BasicMetadata({ metadata }), diff --git a/packages/next/src/lib/metadata/resolve-metadata.ts b/packages/next/src/lib/metadata/resolve-metadata.ts index e8968e974e245..e46a12cecae16 100644 --- a/packages/next/src/lib/metadata/resolve-metadata.ts +++ b/packages/next/src/lib/metadata/resolve-metadata.ts @@ -15,6 +15,7 @@ import { resolveTitle } from './resolvers/resolve-title' import { resolveAsArrayOrUndefined } from './generate/utils' import { isClientReference } from '../client-reference' import { + getErrorOrLayoutModule, getLayoutOrPageModule, LoaderTree, } from '../../server/lib/app-dir-module' @@ -248,28 +249,28 @@ function merge({ async function getDefinedMetadata( mod: any, props: any, - route: string + tracingProps: { route: string } ): Promise { // Layer is a client component, we just skip it. It can't have metadata exported. // Return early to avoid accessing properties error for client references. if (isClientReference(mod)) { return null } - return ( - (mod.generateMetadata - ? (parent: ResolvingMetadata) => - getTracer().trace( - ResolveMetadataSpan.generateMetadata, - { - spanName: `generateMetadata ${route}`, - attributes: { - 'next.page': route, - }, - }, - () => mod.generateMetadata(props, parent) - ) - : mod.metadata) || null - ) + if (typeof mod.generateMetadata === 'function') { + const { route } = tracingProps + return (parent: ResolvingMetadata) => + getTracer().trace( + ResolveMetadataSpan.generateMetadata, + { + spanName: `generateMetadata ${route}`, + attributes: { + 'next.page': route, + }, + }, + () => mod.generateMetadata(props, parent) + ) + } + return mod.metadata || null } async function collectStaticImagesFiles( @@ -317,13 +318,22 @@ export async function collectMetadata({ metadataItems: array, props, route, + errorConvention, }: { tree: LoaderTree metadataItems: MetadataItems props: any route: string + errorConvention?: 'not-found' }) { - const [mod, modType] = await getLayoutOrPageModule(tree) + let mod + let modType + if (errorConvention) { + mod = await getErrorOrLayoutModule(tree, errorConvention) + modType = errorConvention + } else { + ;[mod, modType] = await getLayoutOrPageModule(tree) + } if (modType) { route += `/${modType}` @@ -331,7 +341,7 @@ export async function collectMetadata({ const staticFilesMetadata = await resolveStaticMetadata(tree[2], props) const metadataExport = mod - ? await getDefinedMetadata(mod, props, route) + ? await getDefinedMetadata(mod, props, { route }) : null array.push([metadataExport, staticFilesMetadata]) @@ -344,6 +354,7 @@ export async function resolveMetadata({ treePrefix = [], getDynamicParamFromSegment, searchParams, + errorConvention, }: { tree: LoaderTree parentParams: { [key: string]: any } @@ -352,10 +363,12 @@ export async function resolveMetadata({ treePrefix?: string[] getDynamicParamFromSegment: GetDynamicParamFromSegment searchParams: { [key: string]: any } + errorConvention: 'not-found' | undefined }): Promise { const [segment, parallelRoutes, { page }] = tree const currentTreePrefix = [...treePrefix, segment] const isPage = typeof page !== 'undefined' + // Handle dynamic segment params. const segmentParam = getDynamicParamFromSegment(segment) /** @@ -379,6 +392,7 @@ export async function resolveMetadata({ await collectMetadata({ tree, metadataItems, + errorConvention, props: layerProps, route: currentTreePrefix // __PAGE__ shouldn't be shown in a route @@ -395,6 +409,7 @@ export async function resolveMetadata({ treePrefix: currentTreePrefix, searchParams, getDynamicParamFromSegment, + errorConvention, }) } diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 752eca8ec9e68..f7d6eef26efec 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -83,8 +83,6 @@ import { ModuleReference } from '../../build/webpack/loaders/metadata/types' export const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' -const emptyLoaderTree: LoaderTree = ['', {}, {}] - export type GetDynamicParamFromSegment = ( // [slug] / [[slug]] / [...slug] segment: string @@ -1361,12 +1359,13 @@ export async function renderToHTMLOrFlight( query ) - const createMetadata = (tree: LoaderTree) => ( + const createMetadata = (tree: LoaderTree, errorType?: 'not-found') => ( // Adding key={requestId} to make metadata remount for each render // @ts-expect-error allow to use async server component {createMetadata(loaderTree)}} + initialHead={<>{createMetadata(loaderTree, undefined)}} globalErrorComponent={GlobalError} notFound={ NotFound ? ( - {createMetadata(loaderTree)} + {createMetadata(loaderTree, 'not-found')} {notFoundStyles} @@ -1594,7 +1593,9 @@ export async function renderToHTMLOrFlight( if (isNotFoundError(err)) { res.statusCode = 404 } + let hasRedirectError = false if (isRedirectError(err)) { + hasRedirectError = true res.statusCode = 307 if (err.mutableCookies) { const headers = new Headers() @@ -1609,7 +1610,7 @@ export async function renderToHTMLOrFlight( } const use404Error = res.statusCode === 404 - const useDefaultError = res.statusCode < 400 || res.statusCode === 307 + const useDefaultError = res.statusCode < 400 || hasRedirectError const { layout } = loaderTree[2] const injectedCSS = new Set() @@ -1624,19 +1625,28 @@ export async function renderToHTMLOrFlight( ? interopDefault(await rootLayoutModule()) : null + const metadata = ( + // @ts-expect-error allow to use async server component + + ) const serverErrorElement = ( - } + // For default error we render metadata directly into the head + head={useDefaultError ? metadata : null} > {useDefaultError ? null @@ -1645,6 +1655,8 @@ export async function renderToHTMLOrFlight( async () => { return ( <> + {/* For server components error metadata needs to be inside inline flight data, so they can be hydrated */} + {metadata} {use404Error ? ( {notFoundStyles} diff --git a/packages/next/src/server/lib/app-dir-module.ts b/packages/next/src/server/lib/app-dir-module.ts index 1e348d750a39c..02e735a3edf7a 100644 --- a/packages/next/src/server/lib/app-dir-module.ts +++ b/packages/next/src/server/lib/app-dir-module.ts @@ -32,3 +32,17 @@ export async function getLayoutOrPageModule(loaderTree: LoaderTree) { return [value, modType] as const } + +// First check not-found, if it doesn't exist then pick layout +export async function getErrorOrLayoutModule( + loaderTree: LoaderTree, + errorType: 'error' | 'not-found' +) { + const { [errorType]: error, layout } = loaderTree[2] + if (typeof error !== 'undefined') { + return await error[0]() + } else if (typeof layout !== 'undefined') { + return await layout[0]() + } + return undefined +} diff --git a/test/e2e/app-dir/metadata/app/async/not-found/not-found.tsx b/test/e2e/app-dir/metadata/app/async/not-found/not-found.tsx new file mode 100644 index 0000000000000..8a141c406d44b --- /dev/null +++ b/test/e2e/app-dir/metadata/app/async/not-found/not-found.tsx @@ -0,0 +1,8 @@ +export default function notFound() { + return

local found boundary

+} + +export const metadata = { + title: 'Local not found', + description: 'Local not found description', +} diff --git a/test/e2e/app-dir/metadata/app/layout.tsx b/test/e2e/app-dir/metadata/app/layout.tsx index e427799cce6f7..d28ae1763bc19 100644 --- a/test/e2e/app-dir/metadata/app/layout.tsx +++ b/test/e2e/app-dir/metadata/app/layout.tsx @@ -10,4 +10,5 @@ export default function Layout({ children }) { export const metadata = { title: 'this is the layout title', description: 'this is the layout description', + keywords: ['nextjs', 'react'], } diff --git a/test/e2e/app-dir/metadata/app/not-found.tsx b/test/e2e/app-dir/metadata/app/not-found.tsx index 6030ea22604bd..10bf3ef17b381 100644 --- a/test/e2e/app-dir/metadata/app/not-found.tsx +++ b/test/e2e/app-dir/metadata/app/not-found.tsx @@ -1,3 +1,8 @@ export default function notFound() { return

root not found page

} + +export const metadata = { + title: 'Root not found', + description: 'Root not found description', +} diff --git a/test/e2e/app-dir/metadata/metadata.test.ts b/test/e2e/app-dir/metadata/metadata.test.ts index 5d14658d1d424..72ac4effb512f 100644 --- a/test/e2e/app-dir/metadata/metadata.test.ts +++ b/test/e2e/app-dir/metadata/metadata.test.ts @@ -416,56 +416,6 @@ createNextDescribe( ) }) - it('should render root not-found with default metadata', async () => { - const $ = await next.render$('/does-not-exist') - - // Should contain default metadata and noindex tag - const matchHtml = createMultiHtmlMatcher($) - expect($('meta[charset="utf-8"]').length).toBe(1) - await matchHtml('meta', 'name', 'content', { - viewport: 'width=device-width, initial-scale=1', - robots: 'noindex', - }) - }) - - it('should support notFound in generateMetadata', async () => { - const res = await next.fetch('/async/not-found') - expect(res.status).toBe(404) - const html = await res.text() - const $ = cheerio.load(html) - - // TODO-APP: support render custom not-found in SSR for generateMetadata. - // Check contains root not-found payload in flight response for now. - let hasRootNotFoundFlight = false - for (const el of $('script').toArray()) { - const text = $(el).text() - if (text.includes('root not found page')) { - hasRootNotFoundFlight = true - } - } - expect(hasRootNotFoundFlight).toBe(true) - - // Should contain default metadata and noindex tag - const matchHtml = createMultiHtmlMatcher($) - expect($('meta[charset="utf-8"]').length).toBe(1) - await matchHtml('meta', 'name', 'content', { - viewport: 'width=device-width, initial-scale=1', - robots: 'noindex', - }) - - const browser = await next.browser('/async/not-found') - expect(await browser.elementByCss('h2').text()).toBe( - 'root not found page' - ) - }) - - it('should support redirect in generateMetadata', async () => { - const res = await next.fetch('/async/redirect', { - redirect: 'manual', - }) - expect(res.status).toBe(307) - }) - it('should handle metadataBase for urls resolved as only URL type', async () => { // including few urls in opengraph and alternates const url$ = await next.render$('/metadata-base/url') @@ -593,6 +543,71 @@ createNextDescribe( }) }) + describe('navigation', () => { + it('should render root not-found with default metadata', async () => { + const $ = await next.render$('/does-not-exist') + + // Should contain default metadata and noindex tag + const matchHtml = createMultiHtmlMatcher($) + expect($('meta[charset="utf-8"]').length).toBe(1) + await matchHtml('meta', 'name', 'content', { + viewport: 'width=device-width, initial-scale=1', + robots: 'noindex', + // not found metadata + description: 'Root not found description', + }) + expect(await $('title').text()).toBe('Root not found') + }) + + it('should support notFound in generateMetadata', async () => { + const res = await next.fetch('/async/not-found') + expect(res.status).toBe(404) + const html = await res.text() + const $ = cheerio.load(html) + + // TODO-APP: support render custom not-found in SSR for generateMetadata. + // Check contains root not-found payload in flight response for now. + let hasRootNotFoundFlight = false + for (const el of $('script').toArray()) { + const text = $(el).text() + if (text.includes('root not found page')) { + hasRootNotFoundFlight = true + } + } + expect(hasRootNotFoundFlight).toBe(true) + + // Should contain default metadata and noindex tag + const matchHtml = createMultiHtmlMatcher($) + expect($('meta[charset="utf-8"]').length).toBe(1) + await matchHtml('meta', 'name', 'content', { + viewport: 'width=device-width, initial-scale=1', + robots: 'noindex', + }) + + const browser = await next.browser('/async/not-found') + expect(await browser.elementByCss('h2').text()).toBe( + 'root not found page' + ) + + const matchMultiDom = createMultiDomMatcher(browser) + await matchMultiDom('meta', 'name', 'content', { + viewport: 'width=device-width, initial-scale=1', + keywords: 'parent', + robots: 'noindex', + // not found metadata + description: 'Local not found description', + }) + expect(await getTitle(browser)).toBe('Local not found') + }) + + it('should support redirect in generateMetadata', async () => { + const res = await next.fetch('/async/redirect', { + redirect: 'manual', + }) + expect(res.status).toBe(307) + }) + }) + describe('icons', () => { it('should support basic object icons field', async () => { const browser = await next.browser('/icons')