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

Provide default metadataBase for local and vercel deployment #47568

Merged
merged 12 commits into from
Mar 30, 2023
9 changes: 7 additions & 2 deletions packages/next/src/lib/metadata/default-metadata.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import React from 'react'
import type { ResolvedMetadata } from './types/metadata-interface'

export const createDefaultMetadata = (): ResolvedMetadata => {
export function createDefaultMetadata(): ResolvedMetadata {
const defaultMetadataBase =
process.env.NODE_ENV === 'production' && process.env.VERCEL_URL
? new URL(`https://${process.env.VERCEL_URL}`)
Copy link
Member

Choose a reason for hiding this comment

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

Note that VERCEL_URL will not be the domain visited but will be the unique url. Are you sure you want to expose this by default? It could allow someone to use a pinned version of your application instead of the intended domain that is always latest.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, especially when you're developing a og card, or tweaking around other metadata assets, you want to see if that preview url is working based on your changes instead of always requesting the prod major version

: null
huozhi marked this conversation as resolved.
Show resolved Hide resolved

return {
viewport: 'width=device-width, initial-scale=1',
metadataBase: defaultMetadataBase,

// Other values are all null
metadataBase: null,
title: null,
description: null,
applicationName: null,
Expand Down
6 changes: 5 additions & 1 deletion packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ function merge(
openGraph: string | null
}
) {
const metadataBase = source?.metadataBase || target.metadataBase
// If there's override metadata, prefer it otherwise fallback to the default metadata.
const metadataBase =
typeof source?.metadataBase !== 'undefined'
? source.metadataBase
: source?.metadataBase || target.metadataBase
huozhi marked this conversation as resolved.
Show resolved Hide resolved
for (const key_ in source) {
const key = key_ as keyof Metadata

Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/lib/metadata/resolvers/resolve-opengraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ function resolveImages(
resolvedImages?.forEach((item, index, array) => {
if (isStringOrURL(item)) {
array[index] = {
url: metadataBase ? resolveUrl(item, metadataBase)! : item,
url: resolveUrl(item, metadataBase)!,
}
} else {
// Update image descriptor url
item.url = metadataBase ? resolveUrl(item.url, metadataBase)! : item.url
item.url = resolveUrl(item.url, metadataBase)!
}
})
return resolvedImages
Expand Down
18 changes: 14 additions & 4 deletions packages/next/src/lib/metadata/resolvers/resolve-url.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import path from '../../../shared/lib/isomorphic/path'
import { warnOnce } from '../../../shared/lib/utils/warn-once'

function isStringOrURL(icon: any): icon is string | URL {
return typeof icon === 'string' || icon instanceof URL
Expand All @@ -17,10 +18,19 @@ function resolveUrl(
return parsedUrl
} catch (_) {}

if (!metadataBase)
throw new Error(
`metadata.metadataBase needs to be provided for resolving absolute URLs: ${url}`
)
if (!metadataBase) {
huozhi marked this conversation as resolved.
Show resolved Hide resolved
if (process.env.NODE_ENV !== 'production') {
metadataBase = new URL(`http://localhost:${process.env.PORT || 3000}`)
// Development mode warning
warnOnce(
`metadata.metadataBase is not set and fallbacks to "${metadataBase.origin}", please specify it in root layout to resolve absolute urls.`
)
} else {
throw new Error(
`metadata.metadataBase needs to be provided for resolving absolute URL: ${url}`
)
}
}

// Handle relative or absolute paths
const basePath = metadataBase.pathname || '/'
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata-dynamic-routes/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export default function Layout({ children }) {
}

export const metadata = {
metadataBase: process.env.VERCEL_URL
? new URL(`https://${process.env.VERCEL_URL}`)
: new URL(`http://localhost:${process.env.PORT || 3000}`),
title: 'Next.js App',
description: 'This is a Next.js App',
twitter: {
Expand Down
1 change: 0 additions & 1 deletion test/e2e/app-dir/metadata-dynamic-routes/app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ export default function Page() {
}

export const metadata = {
metadataBase: new URL('https://deploy-preview-abc.vercel.app'),
title: 'index page',
}
21 changes: 12 additions & 9 deletions test/e2e/app-dir/metadata-dynamic-routes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ createNextDescribe(
'app dir - metadata dynamic routes',
{
files: __dirname,
skipDeployment: true,
dependencies: {
'@vercel/og': '0.4.1',
},
},
({ next, isNextDev }) => {
({ next, isNextDev, isNextDeploy }) => {
describe('text routes', () => {
it('should handle robots.[ext] dynamic routes', async () => {
const res = await next.fetch('/robots.txt')
Expand Down Expand Up @@ -172,14 +171,18 @@ createNextDescribe(
expect(twitterTitle).toBe('Twitter - Next.js App')
expect(twitterDescription).toBe('Twitter - This is a Next.js App')

// absolute urls
expect(ogImageUrl).toContain(
`https://deploy-preview-abc.vercel.app/opengraph-image`
)
if (isNextDeploy) {
// absolute urls
expect(ogImageUrl).toMatch(/https:\/\/\w+.vercel.app\/opengraph-image/)
expect(twitterImageUrl).toMatch(
/https:\/\/\w+.vercel.app\/twitter-image/
)
} else {
// absolute urls
expect(ogImageUrl).toMatch(/http:\/\/localhost:\d+\/opengraph-image/)
expect(twitterImageUrl).toMatch(/http:\/\/localhost:\d+\/twitter-image/)
}
expect(ogImageUrl).toMatch(hashRegex)
expect(twitterImageUrl).toContain(
`https://deploy-preview-abc.vercel.app/twitter-image`
)
expect(twitterImageUrl).toMatch(hashRegex)

// alt text
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/metadata-missing-metadata-base/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export default function Layout({ children }) {
return (
<html>
<head></head>
<body>{children}</body>
</html>
)
}

export const metadata = {
title: 'Next.js App',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { ImageResponse } from '@vercel/og'

export const alt = 'Open Graph'

export default function og() {
return new ImageResponse(
(
<div
style={{
width: '100%',
height: '100%',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
fontSize: 128,
background: 'lavender',
}}
>
Open Graph
</div>
)
)
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/metadata-missing-metadata-base/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import React from 'react'

export default function Page() {
return <>hello index</>
}

export const metadata = {
title: 'index page',
}
67 changes: 67 additions & 0 deletions test/e2e/app-dir/metadata-missing-metadata-base/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { fetchViaHTTP } from 'next-test-utils'

describe('app dir - metadata missing metadataBase', () => {
let next: NextInstance
// const logs = []

beforeAll(async () => {
next = await createNext({
skipStart: true,
dependencies: {
'@vercel/og': 'latest',
},
files: new FileRef(__dirname),
})
// next.on('stderr', (log) => {
// logs.push(log)
// })
})
afterAll(() => next.destroy())

if (globalThis.isNextDev) {
it('should warning in development', async () => {
await next.start()
await fetchViaHTTP(next.url, '/')
expect(next.cliOutput).toInclude(
'metadata.metadataBase is not set and fallbacks to "http://localhost:'
)
expect(next.cliOutput).toInclude(
'please specify it in root layout to resolve absolute urls.'
)
})
} else {
it('should error in production', async () => {
await expect(next.start()).rejects.toThrow('next build failed')
expect(next.cliOutput).toInclude(
'metadata.metadataBase needs to be provided for resolving absolute URL: /opengraph-image?'
)
})
}
})

// createNextDescribe(
// 'app dir - metadata missing metadataBase',
// {
// files: __dirname,
// dependencies: {
// '@vercel/og': '0.4.1',
// },
// },
// ({ next, isNextDev, isNextStart }) => {

// if (isNextDev) {
// console.log('logs', logs)
// expect(
// logs.some((log) =>
// /metadata\.metadataBase is not set and fallbacks to "http:\/\/localhost:\d+", please specify it in root layout to resolve absolute urls/.test(
// log
// )
// )
// ).toBe(true)
// } else if (isNextStart) {

// }
// }
// )
huozhi marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
experimental: { appDir: true },
}
5 changes: 3 additions & 2 deletions test/e2e/app-dir/metadata/app/alternate/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export default function Page() {
}

export const metadata = {
metadataBase: null,
alternates: {
canonical: 'https://example.com',
languages: {
Expand All @@ -14,8 +15,8 @@ export const metadata = {
},
types: {
'application/rss+xml': [
{ url: 'blog.rss', title: 'rss' },
{ url: 'blog/js.rss', title: 'js title' },
{ url: '/blog.rss', title: 'rss' },
{ url: '/blog/js.rss', title: 'js title' },
],
},
},
Expand Down
16 changes: 10 additions & 6 deletions test/e2e/app-dir/metadata/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,22 @@ createNextDescribe(
async function checkMeta(
browser: BrowserInterface,
queryValue: string,
expected: string | string[] | undefined | null,
expected: RegExp | string | string[] | undefined | null,
queryKey: string = 'property',
tag: string = 'meta',
domAttributeField: string = 'content'
) {
const values = await browser.eval(
`[...document.querySelectorAll('${tag}[${queryKey}="${queryValue}"]')].map((el) => el.getAttribute("${domAttributeField}"))`
)
if (Array.isArray(expected)) {
expect(values).toEqual(expected)
if (expected instanceof RegExp) {
expect(values[0]).toMatch(expected)
} else {
expect(values[0]).toBe(expected)
if (Array.isArray(expected)) {
expect(values).toEqual(expected)
} else {
expect(values[0]).toBe(expected)
}
}
}

Expand Down Expand Up @@ -286,11 +290,11 @@ createNextDescribe(

await matchDom('link', 'title="js title"', {
type: 'application/rss+xml',
href: 'blog/js.rss',
href: '/blog/js.rss',
})
await matchDom('link', 'title="rss"', {
type: 'application/rss+xml',
href: 'blog.rss',
href: '/blog.rss',
})
})

Expand Down