From 34c2a05da2a3b45ae0529a632ca8b81aee5e81e2 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sat, 1 Jun 2024 20:15:01 +0200 Subject: [PATCH] Warn metadataBase missing in standalone mode or non vercel deployment (#66296) ### What Change the metadataBase missing warning for all cases to only warn in standalone mode or the non-vercel deployment. ### Why In vercel deployments, previous concern was that you might not discover you missed that metadataBase when you deploy. But now we have sth fallback on production deployments. So we only need to warn in non-vercel deployment. Standalone is usually for self-hoist, we always warn users to set the `metadataBase` to make sure the domain can be properly resolved. [x-ref](https://vercel.slack.com/archives/C03S8ED1DKM/p1716926825853389?thread_ts=1716923373.484329&cid=C03S8ED1DKM) --- packages/next/src/lib/metadata/metadata.tsx | 29 ++++++---- .../src/lib/metadata/resolve-metadata.test.ts | 1 + .../next/src/lib/metadata/resolve-metadata.ts | 8 ++- .../resolvers/resolve-opengraph.test.ts | 4 +- .../metadata/resolvers/resolve-opengraph.ts | 38 +++++++++---- .../metadata/resolvers/resolve-url.test.ts | 2 + .../next/src/lib/metadata/types/resolvers.ts | 1 + .../next/src/server/app-render/app-render.tsx | 14 ++--- .../app-dir/metadata-warnings/index.test.ts | 54 +++++++++++++++---- .../metadata-warnings/standalone.test.ts | 2 + 10 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 test/e2e/app-dir/metadata-warnings/standalone.test.ts diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx index 35be61e61b74b..3808f9843e23d 100644 --- a/packages/next/src/lib/metadata/metadata.tsx +++ b/packages/next/src/lib/metadata/metadata.tsx @@ -1,5 +1,8 @@ import type { ParsedUrlQuery } from 'querystring' -import type { GetDynamicParamFromSegment } from '../../server/app-render/app-render' +import type { + AppRenderContext, + GetDynamicParamFromSegment, +} from '../../server/app-render/app-render' import type { LoaderTree } from '../../server/lib/app-dir-module' import React from 'react' @@ -29,6 +32,18 @@ import { createDefaultViewport, } from './default-metadata' import { isNotFoundError } from '../../client/components/not-found' +import type { MetadataContext } from './types/resolvers' + +export function createMetadataContext( + urlPathname: string, + renderOpts: AppRenderContext['renderOpts'] +): MetadataContext { + return { + pathname: urlPathname.split('?')[0], + trailingSlash: renderOpts.trailingSlash, + isStandaloneMode: renderOpts.nextConfigOutput === 'standalone', + } +} // Use a promise to share the status of the metadata resolving, // returning two components `MetadataTree` and `MetadataOutlet` @@ -38,18 +53,16 @@ import { isNotFoundError } from '../../client/components/not-found' // and the error will be caught by the error boundary and trigger fallbacks. export function createMetadataComponents({ tree, - pathname, - trailingSlash, query, + metadataContext, getDynamicParamFromSegment, appUsingSizeAdjustment, errorType, createDynamicallyTrackedSearchParams, }: { tree: LoaderTree - pathname: string - trailingSlash: boolean query: ParsedUrlQuery + metadataContext: MetadataContext getDynamicParamFromSegment: GetDynamicParamFromSegment appUsingSizeAdjustment: boolean errorType?: 'not-found' | 'redirect' @@ -57,12 +70,6 @@ export function createMetadataComponents({ searchParams: ParsedUrlQuery ) => ParsedUrlQuery }): [React.ComponentType, React.ComponentType] { - const metadataContext = { - // Make sure the pathname without query string - pathname: pathname.split('?')[0], - trailingSlash, - } - let resolve: (value: Error | undefined) => void | undefined // Only use promise.resolve here to avoid unhandled rejections const metadataErrorResolving = new Promise((res) => { diff --git a/packages/next/src/lib/metadata/resolve-metadata.test.ts b/packages/next/src/lib/metadata/resolve-metadata.test.ts index 456976c2e4002..5acf5cc0b250e 100644 --- a/packages/next/src/lib/metadata/resolve-metadata.test.ts +++ b/packages/next/src/lib/metadata/resolve-metadata.test.ts @@ -17,6 +17,7 @@ function accumulateMetadata(metadataItems: MetadataItems) { return originAccumulateMetadata(fullMetadataItems, { pathname: '/test', trailingSlash: false, + isStandaloneMode: false, }) } diff --git a/packages/next/src/lib/metadata/resolve-metadata.ts b/packages/next/src/lib/metadata/resolve-metadata.ts index ca6d6685ff8db..547f56cf01a20 100644 --- a/packages/next/src/lib/metadata/resolve-metadata.ts +++ b/packages/next/src/lib/metadata/resolve-metadata.ts @@ -120,6 +120,7 @@ function mergeStaticMetadata( const resolvedTwitter = resolveTwitter( { ...target.twitter, images: twitter } as Twitter, target.metadataBase, + metadataContext, titleTemplates.twitter ) target.twitter = resolvedTwitter @@ -192,6 +193,7 @@ function mergeMetadata({ target.twitter = resolveTwitter( source.twitter, metadataBase, + metadataContext, titleTemplates.twitter ) break @@ -566,7 +568,8 @@ function inheritFromMetadata( const commonOgKeys = ['title', 'description', 'images'] as const function postProcessMetadata( metadata: ResolvedMetadata, - titleTemplates: TitleTemplates + titleTemplates: TitleTemplates, + metadataContext: MetadataContext ): ResolvedMetadata { const { openGraph, twitter } = metadata @@ -599,6 +602,7 @@ function postProcessMetadata( const partialTwitter = resolveTwitter( autoFillProps, metadata.metadataBase, + metadataContext, titleTemplates.twitter ) if (metadata.twitter) { @@ -778,7 +782,7 @@ export async function accumulateMetadata( } } - return postProcessMetadata(resolvedMetadata, titleTemplates) + return postProcessMetadata(resolvedMetadata, titleTemplates, metadataContext) } export async function accumulateViewport( diff --git a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.test.ts b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.test.ts index 822b98023f082..7155723b7555f 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.test.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.test.ts @@ -7,7 +7,7 @@ describe('resolveImages', () => { it(`should resolve images`, () => { const images = [image1, { url: image2, alt: 'Image2' }] - expect(resolveImages(images, null)).toEqual([ + expect(resolveImages(images, null, false)).toEqual([ { url: new URL(image1) }, { url: new URL(image2), alt: 'Image2' }, ]) @@ -16,7 +16,7 @@ describe('resolveImages', () => { it('should not mutate passed images', () => { const images = [image1, { url: image2, alt: 'Image2' }] - resolveImages(images, null) + resolveImages(images, null, false) expect(images).toEqual([image1, { url: image2, alt: 'Image2' }]) }) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts index 61209b8464d06..780f3c2e45ab0 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts @@ -42,14 +42,20 @@ const OgTypeFields = { function resolveAndValidateImage( item: FlattenArray, metadataBase: NonNullable, - isMetadataBaseMissing: boolean + isMetadataBaseMissing: boolean, + isStandaloneMode: boolean ) { if (!item) return undefined const isItemUrl = isStringOrURL(item) const inputUrl = isItemUrl ? item : item.url if (!inputUrl) return undefined - validateResolvedImageUrl(inputUrl, metadataBase, isMetadataBaseMissing) + const isNonVercelDeployment = + !process.env.VERCEL && process.env.NODE_ENV === 'production' + // Validate url in self-host standalone mode or non-Vercel deployment + if (isStandaloneMode || isNonVercelDeployment) { + validateResolvedImageUrl(inputUrl, metadataBase, isMetadataBaseMissing) + } return isItemUrl ? { @@ -64,15 +70,18 @@ function resolveAndValidateImage( export function resolveImages( images: Twitter['images'], - metadataBase: ResolvedMetadataBase + metadataBase: ResolvedMetadataBase, + isStandaloneMode: boolean ): NonNullable['images'] export function resolveImages( images: OpenGraph['images'], - metadataBase: ResolvedMetadataBase + metadataBase: ResolvedMetadataBase, + isStandaloneMode: boolean ): NonNullable['images'] export function resolveImages( images: OpenGraph['images'] | Twitter['images'], - metadataBase: ResolvedMetadataBase + metadataBase: ResolvedMetadataBase, + isStandaloneMode: boolean ): | NonNullable['images'] | NonNullable['images'] { @@ -86,7 +95,8 @@ export function resolveImages( const resolvedItem = resolveAndValidateImage( item, fallbackMetadataBase, - isMetadataBaseMissing + isMetadataBaseMissing, + isStandaloneMode ) if (!resolvedItem) continue @@ -149,7 +159,11 @@ export const resolveOpenGraph: FieldResolverExtraArgs< } } } - target.images = resolveImages(og.images, metadataBase) + target.images = resolveImages( + og.images, + metadataBase, + metadataContext.isStandaloneMode + ) } const resolved = { @@ -179,8 +193,8 @@ const TwitterBasicInfoKeys = [ export const resolveTwitter: FieldResolverExtraArgs< 'twitter', - [ResolvedMetadataBase, string | null] -> = (twitter, metadataBase, titleTemplate) => { + [ResolvedMetadataBase, MetadataContext, string | null] +> = (twitter, metadataBase, metadataContext, titleTemplate) => { if (!twitter) return null let card = 'card' in twitter ? twitter.card : undefined const resolved = { @@ -191,7 +205,11 @@ export const resolveTwitter: FieldResolverExtraArgs< resolved[infoKey] = twitter[infoKey] || null } - resolved.images = resolveImages(twitter.images, metadataBase) + resolved.images = resolveImages( + twitter.images, + metadataBase, + metadataContext.isStandaloneMode + ) card = card || (resolved.images?.length ? 'summary_large_image' : 'summary') resolved.card = card diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts index 25b59cee801ab..b9e1caf796aa5 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts @@ -53,6 +53,7 @@ describe('resolveAbsoluteUrlWithPathname', () => { const opts = { trailingSlash: false, pathname: '/', + isStandaloneMode: false, } const resolver = (url: string | URL) => resolveAbsoluteUrlWithPathname(url, metadataBase, opts) @@ -68,6 +69,7 @@ describe('resolveAbsoluteUrlWithPathname', () => { const opts = { trailingSlash: true, pathname: '/', + isStandaloneMode: false, } const resolver = (url: string | URL) => resolveAbsoluteUrlWithPathname(url, metadataBase, opts) diff --git a/packages/next/src/lib/metadata/types/resolvers.ts b/packages/next/src/lib/metadata/types/resolvers.ts index 15094b6248b76..2ba2cf3182881 100644 --- a/packages/next/src/lib/metadata/types/resolvers.ts +++ b/packages/next/src/lib/metadata/types/resolvers.ts @@ -16,4 +16,5 @@ export type FieldResolverExtraArgs< export type MetadataContext = { pathname: string trailingSlash: boolean + isStandaloneMode: boolean } diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index b0bcd3b8bb7df..208d5c3b19900 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -42,7 +42,10 @@ import { NEXT_URL, RSC_HEADER, } from '../../client/components/app-router-headers' -import { createMetadataComponents } from '../../lib/metadata/metadata' +import { + createMetadataComponents, + createMetadataContext, +} from '../../lib/metadata/metadata' import { RequestAsyncStorageWrapper } from '../async-storage/request-async-storage-wrapper' import { StaticGenerationAsyncStorageWrapper } from '../async-storage/static-generation-async-storage-wrapper' import { isNotFoundError } from '../../client/components/not-found' @@ -325,9 +328,8 @@ async function generateFlight( if (!options?.skipFlight) { const [MetadataTree, MetadataOutlet] = createMetadataComponents({ tree: loaderTree, - pathname: urlPathname, - trailingSlash: ctx.renderOpts.trailingSlash, query, + metadataContext: createMetadataContext(urlPathname, ctx.renderOpts), getDynamicParamFromSegment, appUsingSizeAdjustment, createDynamicallyTrackedSearchParams, @@ -450,9 +452,8 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { const [MetadataTree, MetadataOutlet] = createMetadataComponents({ tree, errorType: asNotFound ? 'not-found' : undefined, - pathname: urlPathname, - trailingSlash: ctx.renderOpts.trailingSlash, query, + metadataContext: createMetadataContext(urlPathname, ctx.renderOpts), getDynamicParamFromSegment: getDynamicParamFromSegment, appUsingSizeAdjustment: appUsingSizeAdjustment, createDynamicallyTrackedSearchParams, @@ -538,8 +539,7 @@ async function ReactServerError({ const [MetadataTree] = createMetadataComponents({ tree, - pathname: urlPathname, - trailingSlash: ctx.renderOpts.trailingSlash, + metadataContext: createMetadataContext(urlPathname, ctx.renderOpts), errorType, query, getDynamicParamFromSegment, diff --git a/test/e2e/app-dir/metadata-warnings/index.test.ts b/test/e2e/app-dir/metadata-warnings/index.test.ts index ba5ac54070468..8062f34875d5d 100644 --- a/test/e2e/app-dir/metadata-warnings/index.test.ts +++ b/test/e2e/app-dir/metadata-warnings/index.test.ts @@ -4,7 +4,7 @@ const METADATA_BASE_WARN_STRING = 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' describe('app dir - metadata missing metadataBase', () => { - const { next, isNextDev, skipped } = nextTestSetup({ + const { next, isNextDev, isNextDeploy, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, }) @@ -13,6 +13,21 @@ describe('app dir - metadata missing metadataBase', () => { return } + if (process.env.TEST_STANDALONE === '1') { + beforeAll(async () => { + await next.stop() + await next.patchFile( + 'next.config.js', + ` + module.exports = { + output: 'standalone', + } + ` + ) + await next.start() + }) + } + // If it's start mode, we get the whole logs since they're from build process. // If it's development mode, we get the logs after request function getCliOutput(logStartPosition: number) { @@ -28,16 +43,33 @@ describe('app dir - metadata missing metadataBase', () => { }) } - it('should fallback to localhost if metadataBase is missing for absolute urls resolving', async () => { - const logStartPosition = next.cliOutput.length - await next.fetch('/og-image-convention') - const output = getCliOutput(logStartPosition) - expect(output).toInclude(METADATA_BASE_WARN_STRING) - expect(output).toMatch(/using "http:\/\/localhost:\d+/) - expect(output).toInclude( - '. See https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase' - ) - }) + if (process.env.TEST_STANDALONE === '1') { + // Standalone mode should always show the warning + it('should fallback to localhost if metadataBase is missing for absolute urls resolving', async () => { + const logStartPosition = next.cliOutput.length + await next.fetch('/og-image-convention') + const output = getCliOutput(logStartPosition) + + expect(output).toInclude(METADATA_BASE_WARN_STRING) + expect(output).toMatch(/using "http:\/\/localhost:\d+/) + expect(output).toInclude( + '. See https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase' + ) + }) + } else { + // Default output mode + it('should show warning in vercel deployment output in default build output mode', async () => { + const logStartPosition = next.cliOutput.length + await next.fetch('/og-image-convention') + const output = getCliOutput(logStartPosition) + + if (isNextDeploy || isNextDev) { + expect(output).not.toInclude(METADATA_BASE_WARN_STRING) + } else { + expect(output).toInclude(METADATA_BASE_WARN_STRING) + } + }) + } it('should warn for unsupported metadata properties', async () => { const logStartPosition = next.cliOutput.length diff --git a/test/e2e/app-dir/metadata-warnings/standalone.test.ts b/test/e2e/app-dir/metadata-warnings/standalone.test.ts new file mode 100644 index 0000000000000..237e8fad24265 --- /dev/null +++ b/test/e2e/app-dir/metadata-warnings/standalone.test.ts @@ -0,0 +1,2 @@ +process.env.TEST_STANDALONE = '1' +require('./index.test')