diff --git a/packages/next/src/build/webpack/loaders/metadata/discover.ts b/packages/next/src/build/webpack/loaders/metadata/discover.ts index b89e93c32a..7f74b0256d 100644 --- a/packages/next/src/build/webpack/loaders/metadata/discover.ts +++ b/packages/next/src/build/webpack/loaders/metadata/discover.ts @@ -6,7 +6,6 @@ import type { import path from 'path' import { stringify } from 'querystring' import { STATIC_METADATA_IMAGES } from '../../../../lib/metadata/is-metadata-route' -import { normalizeAppPath } from '../../../../shared/lib/router/utils/app-paths' const METADATA_TYPE = 'metadata' @@ -125,7 +124,6 @@ export async function createStaticMetadataFromRoute( { type, segment, - route: normalizeAppPath(segment), pageExtensions, } )}!${filepath}${METADATA_RESOURCE_QUERY}` diff --git a/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts b/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts index 5de4281e9a..94bee7ce50 100644 --- a/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts @@ -14,7 +14,6 @@ import { imageExtMimeTypeMap } from '../../../lib/mime-type' import { fileExists } from '../../../lib/file-exists' interface Options { - route: string segment: string type: PossibleImageFileNameConvention pageExtensions: string[] @@ -22,7 +21,7 @@ interface Options { async function nextMetadataImageLoader(this: any, content: Buffer) { const options: Options = this.getOptions() - const { type, route, segment, pageExtensions } = options + const { type, segment, pageExtensions } = options const numericSizes = type === 'twitter' || type === 'openGraph' const { resourcePath, rootContext: context } = this const { name: fileNameBase, ext } = path.parse(resourcePath) @@ -47,36 +46,31 @@ async function nextMetadataImageLoader(this: any, content: Buffer) { ) const isDynamicResource = pageExtensions.includes(extension) - const pageRoute = isDynamicResource ? fileNameBase : interpolatedName + const pageSegment = isDynamicResource ? fileNameBase : interpolatedName const hashQuery = contentHash ? '?' + contentHash : '' if (isDynamicResource) { // re-export and spread as `exportedImageData` to avoid non-exported error return `\ - import path from 'next/dist/shared/lib/isomorphic/path' import * as exported from ${JSON.stringify(resourcePath)} - import { interpolateDynamicPath } from 'next/dist/server/server-utils' - import { getNamedRouteRegex } from 'next/dist/shared/lib/router/utils/route-regex' - import { getMetadataRouteSuffix } from 'next/dist/lib/metadata/get-metadata-route' + import { fillMetadataSegment } from 'next/dist/lib/metadata/get-metadata-route' const imageModule = { ...exported } export default async function (props) { - const pathname = ${JSON.stringify(route)} - const routeRegex = getNamedRouteRegex(pathname, false) - const segment = ${JSON.stringify(segment)} const { __metadata_id__: _, ...params } = props.params - const route = interpolateDynamicPath(pathname, params, routeRegex) - const suffix = getMetadataRouteSuffix(segment) - const routeSuffix = suffix ? \`-\${suffix}\` : '' - const imageRoute = ${JSON.stringify(pageRoute)} + routeSuffix + const imageUrl = fillMetadataSegment(${JSON.stringify( + segment + )}, params, ${JSON.stringify(pageSegment)}) const { generateImageMetadata } = imageModule - function getImageMetadata(imageMetadata, segment) { + function getImageMetadata(imageMetadata, idParam) { const data = { alt: imageMetadata.alt, type: imageMetadata.contentType || 'image/png', - url: path.join(route, segment + ${JSON.stringify(hashQuery)}), + url: imageUrl + (idParam ? ('/' + idParam) : '') + ${JSON.stringify( + hashQuery + )}, } const { size } = imageMetadata if (size) { @@ -92,11 +86,11 @@ async function nextMetadataImageLoader(this: any, content: Buffer) { if (generateImageMetadata) { const imageMetadataArray = await generateImageMetadata({ params }) return imageMetadataArray.map((imageMetadata, index) => { - const segment = path.join(imageRoute, (imageMetadata.id || index) + '') - return getImageMetadata(imageMetadata, segment) + const idParam = (imageMetadata.id || index) + '' + return getImageMetadata(imageMetadata, idParam) }) } else { - return [getImageMetadata(imageModule, imageRoute)] + return [getImageMetadata(imageModule, '')] } }` } @@ -137,27 +131,17 @@ async function nextMetadataImageLoader(this: any, content: Buffer) { } return `\ - import path from 'next/dist/shared/lib/isomorphic/path' - import { interpolateDynamicPath } from 'next/dist/server/server-utils' - import { getNamedRouteRegex } from 'next/dist/shared/lib/router/utils/route-regex' - import { getMetadataRouteSuffix } from 'next/dist/lib/metadata/get-metadata-route' + import { fillMetadataSegment } from 'next/dist/lib/metadata/get-metadata-route' export default (props) => { - const pathname = ${JSON.stringify(route)} - const routeRegex = getNamedRouteRegex(pathname, false) - const segment = ${JSON.stringify(segment)} - const route = interpolateDynamicPath(pathname, props.params, routeRegex) - const suffix = getMetadataRouteSuffix(segment) - const routeSuffix = suffix ? \`-\${suffix}\` : '' - const { name, ext } = path.parse(${JSON.stringify(pageRoute)}) - - const imageData = ${JSON.stringify(imageData)}; + const imageData = ${JSON.stringify(imageData)} + const imageUrl = fillMetadataSegment(${JSON.stringify( + segment + )}, props.params, ${JSON.stringify(pageSegment)}) return [{ ...imageData, - url: path.join(route, name + routeSuffix + ext + ${JSON.stringify( - type === 'favicon' ? '' : hashQuery - )}), + url: imageUrl + ${JSON.stringify(type === 'favicon' ? '' : hashQuery)}, }] }` } diff --git a/packages/next/src/build/webpack/loaders/next-metadata-route-loader.ts b/packages/next/src/build/webpack/loaders/next-metadata-route-loader.ts index 32303c6672..da5b07e751 100644 --- a/packages/next/src/build/webpack/loaders/next-metadata-route-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-metadata-route-loader.ts @@ -103,11 +103,20 @@ const generateImageMetadata = imageModule.generateImageMetadata export async function GET(_, ctx) { const { __metadata_id__ = [], ...params } = ctx.params - const id = __metadata_id__[0] + const targetId = __metadata_id__[0] + let id = undefined const imageMetadata = generateImageMetadata ? await generateImageMetadata({ params }) : null + if (imageMetadata) { - const hasId = imageMetadata.some((item) => item.id.toString() === id) - if (!hasId) { + id = imageMetadata.find((item) => { + if (process.env.NODE_ENV !== 'production') { + if (item?.id == null) { + throw new Error('id is required for every item returned from generateImageMetadata') + } + } + return item.id.toString() === targetId + })?.id + if (id == null) { return new NextResponse('Not Found', { status: 404, }) @@ -132,13 +141,20 @@ const contentType = ${JSON.stringify(getContentType(resourcePath))} const fileType = ${JSON.stringify(getFilenameAndExtension(resourcePath).name)} export async function GET(_, ctx) { - const sitemaps = generateSitemaps ? await generateSitemaps() : null + const { __metadata_id__ = [], ...params } = ctx.params + const targetId = __metadata_id__[0] let id = undefined + const sitemaps = generateSitemaps ? await generateSitemaps() : null if (sitemaps) { - const { __metadata_id__ = [] } = ctx.params - const targetId = __metadata_id__[0] - id = sitemaps.find((item) => item.id.toString() === targetId)?.id + id = sitemaps.find((item) => { + if (process.env.NODE_ENV !== 'production') { + if (item?.id == null) { + throw new Error('id property is required for every item returned from generateSitemaps') + } + } + return item.id.toString() === targetId + })?.id if (id == null) { return new NextResponse('Not Found', { status: 404, diff --git a/packages/next/src/lib/metadata/get-metadata-route.ts b/packages/next/src/lib/metadata/get-metadata-route.ts index 497bdd2b3e..b9b8400a49 100644 --- a/packages/next/src/lib/metadata/get-metadata-route.ts +++ b/packages/next/src/lib/metadata/get-metadata-route.ts @@ -1,6 +1,9 @@ import { isMetadataRoute, isMetadataRouteFile } from './is-metadata-route' import path from '../../shared/lib/isomorphic/path' +import { interpolateDynamicPath } from '../../server/server-utils' +import { getNamedRouteRegex } from '../../shared/lib/router/utils/route-regex' import { djb2Hash } from '../../shared/lib/hash' +import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths' /* * If there's special convention like (...) or @ in the page path, @@ -10,7 +13,7 @@ import { djb2Hash } from '../../shared/lib/hash' * /app/open-graph.tsx -> /open-graph/route * /app/(post)/open-graph.tsx -> /open-graph/route-[0-9a-z]{6} */ -export function getMetadataRouteSuffix(page: string) { +function getMetadataRouteSuffix(page: string) { let suffix = '' if ((page.includes('(') && page.includes(')')) || page.includes('@')) { @@ -19,6 +22,29 @@ export function getMetadataRouteSuffix(page: string) { return suffix } +/** + * Fill the dynamic segment in the metadata route + * + * Example: + * fillMetadataSegment('/a/[slug]', { params: { slug: 'b' } }, 'open-graph') -> '/a/b/open-graph' + * + */ +export function fillMetadataSegment( + segment: string, + params: any, + imageSegment: string +) { + const pathname = normalizeAppPath(segment) + const routeRegex = getNamedRouteRegex(pathname, false) + const route = interpolateDynamicPath(pathname, params, routeRegex) + const suffix = getMetadataRouteSuffix(segment) + const routeSuffix = suffix ? `-${suffix}` : '' + + const { name, ext } = path.parse(imageSegment) + + return path.join(route, `${name}${routeSuffix}${ext}`) +} + /** * Map metadata page key to the corresponding route * diff --git a/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts b/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts index 702057a836..da188312f4 100644 --- a/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts +++ b/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts @@ -1,5 +1,6 @@ import { createNextDescribe } from 'e2e-utils' import imageSize from 'image-size' +import { check } from 'next-test-utils' const CACHE_HEADERS = { NONE: 'no-cache, no-store', @@ -108,102 +109,97 @@ createNextDescribe( ) }) - if (!isNextDeploy) { - // TODO-APP: investigate the dynamic routes failing in deployment tests - it('should support generate multi images with generateImageMetadata', async () => { - const $ = await next.render$('/dynamic/big') - const iconUrls = $('link[rel="icon"]') - .toArray() - .map((el) => { - return { - href: $(el).attr('href').split('?')[0], - sizes: $(el).attr('sizes'), - type: $(el).attr('type'), - } - }) - // slug is id param from generateImageMetadata - expect(iconUrls).toMatchObject([ - { - href: '/dynamic/big/icon-48jo90/small', - sizes: '48x48', - type: 'image/png', - }, - { - href: '/dynamic/big/icon-48jo90/medium', - sizes: '72x72', - type: 'image/png', - }, - ]) + it('should support generate multi images with generateImageMetadata', async () => { + const $ = await next.render$('/dynamic/big') + const iconUrls = $('link[rel="icon"]') + .toArray() + .map((el) => { + return { + href: $(el).attr('href').split('?')[0], + sizes: $(el).attr('sizes'), + type: $(el).attr('type'), + } + }) + // slug is id param from generateImageMetadata + expect(iconUrls).toMatchObject([ + { + href: '/dynamic/big/icon-48jo90/small', + sizes: '48x48', + type: 'image/png', + }, + { + href: '/dynamic/big/icon-48jo90/medium', + sizes: '72x72', + type: 'image/png', + }, + ]) - const appleTouchIconUrls = $('link[rel="apple-touch-icon"]') - .toArray() - .map((el) => { - return { - href: $(el).attr('href').split('?')[0], - sizes: $(el).attr('sizes'), - type: $(el).attr('type'), - } - }) - // slug is index by default - expect(appleTouchIconUrls).toEqual([ - { - href: '/dynamic/big/apple-icon-48jo90/0', - sizes: '48x48', - type: 'image/png', - }, - { - href: '/dynamic/big/apple-icon-48jo90/1', - sizes: '64x64', - type: 'image/png', - }, - ]) - }) + const appleTouchIconUrls = $('link[rel="apple-touch-icon"]') + .toArray() + .map((el) => { + return { + href: $(el).attr('href').split('?')[0], + sizes: $(el).attr('sizes'), + type: $(el).attr('type'), + } + }) + // slug is index by default + expect(appleTouchIconUrls).toEqual([ + { + href: '/dynamic/big/apple-icon-48jo90/0', + sizes: '48x48', + type: 'image/png', + }, + { + href: '/dynamic/big/apple-icon-48jo90/1', + sizes: '64x64', + type: 'image/png', + }, + ]) + }) - it('should support generate multi sitemaps with generateSitemaps', async () => { - const ids = [0, 1, 2, 3] - function fetchSitemap(id) { - return next - .fetch(`/dynamic/small/sitemap.xml/${id}`) - .then((res) => res.text()) - } + it('should support generate multi sitemaps with generateSitemaps', async () => { + const ids = [0, 1, 2, 3] + function fetchSitemap(id) { + return next + .fetch(`/dynamic/small/sitemap.xml/${id}`) + .then((res) => res.text()) + } - for (const id of ids) { - const text = await fetchSitemap(id) - expect(text).toContain( - `https://example.com/dynamic/${id}` - ) - } - }) + for (const id of ids) { + const text = await fetchSitemap(id) + expect(text).toContain(`https://example.com/dynamic/${id}`) + } + }) - it('should fill params into dynamic routes url of metadata images', async () => { - const $ = await next.render$('/dynamic/big') - const ogImageUrl = $('meta[property="og:image"]').attr('content') - expect(ogImageUrl).toMatch(hashRegex) - expect(ogImageUrl).toMatch('/dynamic/big/opengraph-image') - // should already normalize the parallel routes segment to url - expect(ogImageUrl).not.toContain('(group)') - }) + it('should fill params into dynamic routes url of metadata images', async () => { + const $ = await next.render$('/dynamic/big') + const ogImageUrl = $('meta[property="og:image"]').attr('content') + expect(ogImageUrl).toMatch(hashRegex) + expect(ogImageUrl).toMatch('/dynamic/big/opengraph-image') + // should already normalize the parallel routes segment to url + expect(ogImageUrl).not.toContain('(group)') + }) - it('should support params as argument in dynamic routes', async () => { - const big$ = await next.render$('/dynamic/big') - const small$ = await next.render$('/dynamic/small') - const bigOgUrl = new URL( - big$('meta[property="og:image"]').attr('content') - ) - const smallOgUrl = new URL( - small$('meta[property="og:image"]').attr('content') - ) - const bufferBig = await (await next.fetch(bigOgUrl.pathname)).buffer() - const bufferSmall = await ( - await next.fetch(smallOgUrl.pathname) - ).buffer() + it('should support params as argument in dynamic routes', async () => { + const big$ = await next.render$('/dynamic/big') + const small$ = await next.render$('/dynamic/small') + const bigOgUrl = new URL( + big$('meta[property="og:image"]').attr('content') + ) + const smallOgUrl = new URL( + small$('meta[property="og:image"]').attr('content') + ) + const bufferBig = await (await next.fetch(bigOgUrl.pathname)).buffer() + const bufferSmall = await ( + await next.fetch(smallOgUrl.pathname) + ).buffer() - const sizeBig = imageSize(bufferBig) - const sizeSmall = imageSize(bufferSmall) - expect([sizeBig.width, sizeBig.height]).toEqual([1200, 630]) - expect([sizeSmall.width, sizeSmall.height]).toEqual([600, 315]) - }) - } + const sizeBig = imageSize(bufferBig) + const sizeSmall = imageSize(bufferSmall) + expect([sizeBig.width, sizeBig.height]).toEqual([1200, 630]) + expect([sizeSmall.width, sizeSmall.height]).toEqual([600, 315]) + }) it('should fill params into routes groups url of static images', async () => { const $ = await next.render$('/static') @@ -347,6 +343,74 @@ createNextDescribe( expect(twitterImage).toMatch(/\/metadata-base\/unset\/twitter-image\.png/) }) + if (isNextDev) { + it('should error when id is missing in generateImageMetadata', async () => { + const iconFilePath = 'app/metadata-base/unset/icon.tsx' + const contentMissingIdProperty = ` + import { ImageResponse } from 'next/server' + export async function generateImageMetadata() { + return [ + { + contentType: 'image/png', + size: { width: 48, height: 48 }, + // id: 100, + }, + { + contentType: 'image/png', + size: { width: 48, height: 48 }, + id: 101, + }, + ] + } + + export default function icon() { + return new ImageResponse(
icon
) + } + ` + await next.patchFile(iconFilePath, contentMissingIdProperty) + await next.fetch('/metadata-base/unset/icon/100') + await next.deleteFile(iconFilePath) // revert + + await check(async () => { + expect(next.cliOutput).toContain( + `id is required for every item returned from generateImageMetadata` + ) + return 'success' + }, /success/) + }) + + it('should error when id is missing in generateSitemaps', async () => { + const sitemapFilePath = 'app/metadata-base/unset/sitemap.tsx' + const contentMissingIdProperty = ` + import { MetadataRoute } from 'next' + + export async function generateSitemaps() { + return [ + { id: 0 }, + ] + } + + export default function sitemap({ id }): MetadataRoute.Sitemap { + return [ + { + url: 'https://example.com/', + lastModified: '2021-01-01', + }, + ] + }` + await next.patchFile(sitemapFilePath, contentMissingIdProperty) + await next.fetch('/metadata-base/unset/sitemap.xml/0') + await next.deleteFile(sitemapFilePath) // revert + + await check(async () => { + expect(next.cliOutput).toContain( + `id is required for every item returned from generateImageMetadata` + ) + return 'success' + }, /success/) + }) + } + if (isNextStart) { it('should support edge runtime of image routes', async () => { const middlewareManifest = JSON.parse(