From 008cecf97142cce7ad678ddde9b147b047a4b883 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 11 Feb 2024 00:19:46 +0100 Subject: [PATCH 1/3] Should not warn metadataBase missing if only absolute urls are present --- .../metadata/resolvers/resolve-opengraph.ts | 87 +++++++++++++----- .../src/lib/metadata/resolvers/resolve-url.ts | 24 ++--- packages/next/src/lib/url.ts | 8 +- .../app/page.js | 9 -- .../next.config.js | 1 - .../app/absolute-url-og/page.js | 9 ++ .../app/layout.js | 1 - .../og-image-convention}/opengraph-image.png | Bin .../app/og-image-convention/page.js | 3 + .../app/unsupported-metadata/page.js | 0 .../index.test.ts | 14 ++- 11 files changed, 102 insertions(+), 54 deletions(-) delete mode 100644 test/e2e/app-dir/metadata-missing-metadata-base/app/page.js delete mode 100644 test/e2e/app-dir/metadata-missing-metadata-base/next.config.js create mode 100644 test/e2e/app-dir/metadata-warnings/app/absolute-url-og/page.js rename test/e2e/app-dir/{metadata-missing-metadata-base => metadata-warnings}/app/layout.js (85%) rename test/e2e/app-dir/{metadata-missing-metadata-base/app => metadata-warnings/app/og-image-convention}/opengraph-image.png (100%) create mode 100644 test/e2e/app-dir/metadata-warnings/app/og-image-convention/page.js rename test/e2e/app-dir/{metadata-missing-metadata-base => metadata-warnings}/app/unsupported-metadata/page.js (100%) rename test/e2e/app-dir/{metadata-missing-metadata-base => metadata-warnings}/index.test.ts (72%) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts index c805203619eb93..dae6fc94ddbbc9 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts @@ -17,6 +17,11 @@ import { resolveAbsoluteUrlWithPathname, } from './resolve-url' import { resolveTitle } from './resolve-title' +import { isFullStringUrl } from '../../url' +import { warnOnce } from '../../../build/output/log' + +type FlattenArray = T extends (infer U)[] ? U : T +type ResolvedMetadataBase = ResolvedMetadata['metadataBase'] const OgTypeFields = { article: ['authors', 'tags'], @@ -34,41 +39,58 @@ const OgTypeFields = { ], } as const +function resolveAndValidateImage( + item: FlattenArray, + metadataBase: NonNullable, + isMetadataBaseMissing: boolean +) { + if (!item) return undefined + const isItemUrl = isStringOrURL(item) + const inputUrl = isItemUrl ? item : item.url + if (!inputUrl) return undefined + + validateResolvedImageUrl(inputUrl, metadataBase, isMetadataBaseMissing) + + return isItemUrl + ? { + url: resolveUrl(inputUrl, metadataBase), + } + : { + ...item, + // Update image descriptor url + url: resolveUrl(inputUrl, metadataBase), + } +} + export function resolveImages( images: Twitter['images'], - metadataBase: ResolvedMetadata['metadataBase'] + metadataBase: ResolvedMetadataBase ): NonNullable['images'] export function resolveImages( images: OpenGraph['images'], - metadataBase: ResolvedMetadata['metadataBase'] + metadataBase: ResolvedMetadataBase ): NonNullable['images'] export function resolveImages( images: OpenGraph['images'] | Twitter['images'], - metadataBase: ResolvedMetadata['metadataBase'] + metadataBase: ResolvedMetadataBase ): | NonNullable['images'] | NonNullable['images'] { const resolvedImages = resolveAsArrayOrUndefined(images) if (!resolvedImages) return resolvedImages + const { isMetadataBaseMissing, fallbackMetadataBase } = + getSocialImageFallbackMetadataBase(metadataBase) const nonNullableImages = [] for (const item of resolvedImages) { - if (!item) continue - const isItemUrl = isStringOrURL(item) - const inputUrl = isItemUrl ? item : item.url - if (!inputUrl) continue - - nonNullableImages.push( - isItemUrl - ? { - url: resolveUrl(item, metadataBase), - } - : { - ...item, - // Update image descriptor url - url: resolveUrl(item.url, metadataBase), - } + const resolvedItem = resolveAndValidateImage( + item, + fallbackMetadataBase, + isMetadataBaseMissing ) + if (!resolvedItem) continue + + nonNullableImages.push(resolvedItem) } return nonNullableImages @@ -94,9 +116,26 @@ function getFieldsByOgType(ogType: OpenGraphType | undefined) { } } +function validateResolvedImageUrl( + inputUrl: string | URL, + fallbackMetadataBase: NonNullable, + isMetadataBaseMissing: boolean +): void { + // Only warn on the image url that needs to be resolved with metadataBase + if ( + typeof inputUrl === 'string' && + !isFullStringUrl(inputUrl) && + isMetadataBaseMissing + ) { + warnOnce( + `metadataBase property in metadata export is not set for resolving social open graph or twitter images, using "${fallbackMetadataBase.origin}". See https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase` + ) + } +} + export const resolveOpenGraph: FieldResolverExtraArgs< 'openGraph', - [ResolvedMetadata['metadataBase'], MetadataContext, string | null] + [ResolvedMetadataBase, MetadataContext, string | null] > = (openGraph, metadataBase, metadataContext, titleTemplate) => { if (!openGraph) return null @@ -114,9 +153,7 @@ export const resolveOpenGraph: FieldResolverExtraArgs< } } } - - const imageMetadataBase = getSocialImageFallbackMetadataBase(metadataBase) - target.images = resolveImages(og.images, imageMetadataBase) + target.images = resolveImages(og.images, metadataBase) } const resolved = { @@ -146,7 +183,7 @@ const TwitterBasicInfoKeys = [ export const resolveTwitter: FieldResolverExtraArgs< 'twitter', - [ResolvedMetadata['metadataBase'], string | null] + [ResolvedMetadataBase, string | null] > = (twitter, metadataBase, titleTemplate) => { if (!twitter) return null let card = 'card' in twitter ? twitter.card : undefined @@ -157,8 +194,8 @@ export const resolveTwitter: FieldResolverExtraArgs< for (const infoKey of TwitterBasicInfoKeys) { resolved[infoKey] = twitter[infoKey] || null } - const imageMetadataBase = getSocialImageFallbackMetadataBase(metadataBase) - resolved.images = resolveImages(twitter.images, imageMetadataBase) + + resolved.images = resolveImages(twitter.images, metadataBase) card = card || (resolved.images?.length ? 'summary_large_image' : 'summary') resolved.card = card diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.ts index a521118292eaf9..8ddc6aa85c36c0 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.ts @@ -1,5 +1,4 @@ import path from '../../../shared/lib/isomorphic/path' -import * as Log from '../../../build/output/log' import type { MetadataContext } from '../types/resolvers' function isStringOrURL(icon: any): icon is string | URL { @@ -12,19 +11,20 @@ function createLocalMetadataBase() { // For deployment url for metadata routes, prefer to use the deployment url if possible // as these routes are unique to the deployments url. -export function getSocialImageFallbackMetadataBase( - metadataBase: URL | null -): URL | null { +export function getSocialImageFallbackMetadataBase(metadataBase: URL | null): { + fallbackMetadataBase: URL + isMetadataBaseMissing: boolean +} { const isMetadataBaseMissing = !metadataBase const defaultMetadataBase = createLocalMetadataBase() const deploymentUrl = process.env.VERCEL_URL && new URL(`https://${process.env.VERCEL_URL}`) - let fallbackMetadata + let fallbackMetadataBase if (process.env.NODE_ENV === 'development') { - fallbackMetadata = defaultMetadataBase + fallbackMetadataBase = defaultMetadataBase } else { - fallbackMetadata = + fallbackMetadataBase = process.env.NODE_ENV === 'production' && deploymentUrl && process.env.VERCEL_ENV === 'preview' @@ -32,14 +32,10 @@ export function getSocialImageFallbackMetadataBase( : metadataBase || deploymentUrl || defaultMetadataBase } - if (isMetadataBaseMissing) { - Log.warnOnce('') - Log.warnOnce( - `metadata.metadataBase is not set for resolving social open graph or twitter images, using "${fallbackMetadata.origin}". See https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase` - ) + return { + fallbackMetadataBase, + isMetadataBaseMissing, } - - return fallbackMetadata } function resolveUrl(url: null | undefined, metadataBase: URL | null): null diff --git a/packages/next/src/lib/url.ts b/packages/next/src/lib/url.ts index 80bb04cd67aed3..32aea560824901 100644 --- a/packages/next/src/lib/url.ts +++ b/packages/next/src/lib/url.ts @@ -1,7 +1,13 @@ +const DUMMY_ORIGIN = 'http://n' + function getUrlWithoutHost(url: string) { - return new URL(url, 'http://n') + return new URL(url, DUMMY_ORIGIN) } export function getPathname(url: string) { return getUrlWithoutHost(url).pathname } + +export function isFullStringUrl(url: string) { + return /https?:\/\//.test(url) +} diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/app/page.js b/test/e2e/app-dir/metadata-missing-metadata-base/app/page.js deleted file mode 100644 index eba737535cc7e9..00000000000000 --- a/test/e2e/app-dir/metadata-missing-metadata-base/app/page.js +++ /dev/null @@ -1,9 +0,0 @@ -import React from 'react' - -export default function Page() { - return <>hello index -} - -export const metadata = { - title: 'index page', -} diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/next.config.js b/test/e2e/app-dir/metadata-missing-metadata-base/next.config.js deleted file mode 100644 index 4ba52ba2c8df67..00000000000000 --- a/test/e2e/app-dir/metadata-missing-metadata-base/next.config.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = {} diff --git a/test/e2e/app-dir/metadata-warnings/app/absolute-url-og/page.js b/test/e2e/app-dir/metadata-warnings/app/absolute-url-og/page.js new file mode 100644 index 00000000000000..3e8b9222002b44 --- /dev/null +++ b/test/e2e/app-dir/metadata-warnings/app/absolute-url-og/page.js @@ -0,0 +1,9 @@ +export default function Page() { + return 'absolute url og page' +} + +export const metadata = { + openGraph: { + images: 'https://repository-images.githubusercontent.com/123', + }, +} diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/app/layout.js b/test/e2e/app-dir/metadata-warnings/app/layout.js similarity index 85% rename from test/e2e/app-dir/metadata-missing-metadata-base/app/layout.js rename to test/e2e/app-dir/metadata-warnings/app/layout.js index 762515029332e8..750eb927b19801 100644 --- a/test/e2e/app-dir/metadata-missing-metadata-base/app/layout.js +++ b/test/e2e/app-dir/metadata-warnings/app/layout.js @@ -1,7 +1,6 @@ export default function Layout({ children }) { return ( - {children} ) diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/app/opengraph-image.png b/test/e2e/app-dir/metadata-warnings/app/og-image-convention/opengraph-image.png similarity index 100% rename from test/e2e/app-dir/metadata-missing-metadata-base/app/opengraph-image.png rename to test/e2e/app-dir/metadata-warnings/app/og-image-convention/opengraph-image.png diff --git a/test/e2e/app-dir/metadata-warnings/app/og-image-convention/page.js b/test/e2e/app-dir/metadata-warnings/app/og-image-convention/page.js new file mode 100644 index 00000000000000..9c4a21008113f0 --- /dev/null +++ b/test/e2e/app-dir/metadata-warnings/app/og-image-convention/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return 'page' +} diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/app/unsupported-metadata/page.js b/test/e2e/app-dir/metadata-warnings/app/unsupported-metadata/page.js similarity index 100% rename from test/e2e/app-dir/metadata-missing-metadata-base/app/unsupported-metadata/page.js rename to test/e2e/app-dir/metadata-warnings/app/unsupported-metadata/page.js diff --git a/test/e2e/app-dir/metadata-missing-metadata-base/index.test.ts b/test/e2e/app-dir/metadata-warnings/index.test.ts similarity index 72% rename from test/e2e/app-dir/metadata-missing-metadata-base/index.test.ts rename to test/e2e/app-dir/metadata-warnings/index.test.ts index 5b9ae08d06c729..27ba3b3e7a7e9f 100644 --- a/test/e2e/app-dir/metadata-missing-metadata-base/index.test.ts +++ b/test/e2e/app-dir/metadata-warnings/index.test.ts @@ -18,11 +18,10 @@ createNextDescribe( it('should fallback to localhost if metadataBase is missing for absolute urls resolving', async () => { const logStartPosition = next.cliOutput.length - await fetchViaHTTP(next.url, '/') - // + await fetchViaHTTP(next.url, '/og-image-convention') const output = getCliOutput(logStartPosition) expect(output).toInclude( - 'metadata.metadataBase is not set for resolving social open graph or twitter images,' + 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' ) expect(output).toMatch(/using "http:\/\/localhost:\d+/) expect(output).toInclude( @@ -30,6 +29,15 @@ createNextDescribe( ) }) + it('should not warn metadataBase is missing if there is only absolute url', async () => { + const logStartPosition = next.cliOutput.length + await fetchViaHTTP(next.url, '/absolute-url-og') + const output = getCliOutput(logStartPosition) + expect(output).not.toInclude( + 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' + ) + }) + it('should warn for unsupported metadata properties', async () => { const logStartPosition = next.cliOutput.length await fetchViaHTTP(next.url, '/unsupported-metadata') From f7e5a89a20d155d56269f4cfb84396b803ec771f Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 11 Feb 2024 00:30:53 +0100 Subject: [PATCH 2/3] share const in test --- test/e2e/app-dir/metadata-warnings/index.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/e2e/app-dir/metadata-warnings/index.test.ts b/test/e2e/app-dir/metadata-warnings/index.test.ts index 27ba3b3e7a7e9f..df3808ea0fc4a8 100644 --- a/test/e2e/app-dir/metadata-warnings/index.test.ts +++ b/test/e2e/app-dir/metadata-warnings/index.test.ts @@ -1,6 +1,9 @@ import { createNextDescribe } from 'e2e-utils' import { fetchViaHTTP } from 'next-test-utils' +const METADATA_BASE_WARN_STRING = + 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' + createNextDescribe( 'app dir - metadata missing metadataBase', { @@ -20,9 +23,7 @@ createNextDescribe( const logStartPosition = next.cliOutput.length await fetchViaHTTP(next.url, '/og-image-convention') const output = getCliOutput(logStartPosition) - expect(output).toInclude( - 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' - ) + 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' @@ -33,9 +34,7 @@ createNextDescribe( const logStartPosition = next.cliOutput.length await fetchViaHTTP(next.url, '/absolute-url-og') const output = getCliOutput(logStartPosition) - expect(output).not.toInclude( - 'metadataBase property in metadata export is not set for resolving social open graph or twitter images,' - ) + expect(output).not.toInclude(METADATA_BASE_WARN_STRING) }) it('should warn for unsupported metadata properties', async () => { From 7ab9abb7455d9849ba15fbd0cfe36e775c601bf3 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 11 Feb 2024 12:13:31 +0100 Subject: [PATCH 3/3] update test --- .../app-dir/metadata-warnings/index.test.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/e2e/app-dir/metadata-warnings/index.test.ts b/test/e2e/app-dir/metadata-warnings/index.test.ts index df3808ea0fc4a8..5fc0facc543d24 100644 --- a/test/e2e/app-dir/metadata-warnings/index.test.ts +++ b/test/e2e/app-dir/metadata-warnings/index.test.ts @@ -10,13 +10,20 @@ createNextDescribe( files: __dirname, skipDeployment: true, }, - ({ next, isNextStart }) => { + ({ next, isNextDev }) => { // If it's start mode, we get the whole logs since they're from build process. // If it's dev mode, we get the logs after request function getCliOutput(logStartPosition: number) { - return isNextStart - ? next.cliOutput - : next.cliOutput.slice(logStartPosition) + return isNextDev ? next.cliOutput.slice(logStartPosition) : next.cliOutput + } + + if (isNextDev) { + it('should not warn metadataBase is missing if there is only absolute url', async () => { + const logStartPosition = next.cliOutput.length + await fetchViaHTTP(next.url, '/absolute-url-og') + const output = getCliOutput(logStartPosition) + expect(output).not.toInclude(METADATA_BASE_WARN_STRING) + }) } it('should fallback to localhost if metadataBase is missing for absolute urls resolving', async () => { @@ -30,13 +37,6 @@ createNextDescribe( ) }) - it('should not warn metadataBase is missing if there is only absolute url', async () => { - const logStartPosition = next.cliOutput.length - await fetchViaHTTP(next.url, '/absolute-url-og') - const output = getCliOutput(logStartPosition) - expect(output).not.toInclude(METADATA_BASE_WARN_STRING) - }) - it('should warn for unsupported metadata properties', async () => { const logStartPosition = next.cliOutput.length await fetchViaHTTP(next.url, '/unsupported-metadata')