Skip to content

Commit

Permalink
Freeze resolved metadata object in dev mode (#45923)
Browse files Browse the repository at this point in the history
In dev mode, instead of `resolve(resolvedMetadata)` for the parent metadata argument we pass down `resolve(freeze(deepClone(resolvedMetadata)))` as parent metdadata, this approach will avoid users mutating resolved metadata manually but still allowing next manage to merge it during resolving

Closes NEXT-559

- [x] linked task
- [x] e2e tests
  • Loading branch information
huozhi authored Feb 15, 2023
1 parent 79da1e7 commit 2b2c746
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 4 deletions.
12 changes: 10 additions & 2 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,13 @@ export async function collectMetadata(
export async function accumulateMetadata(
metadataItems: MetadataItems
): Promise<ResolvedMetadata> {
const deepClone: <T>(v: T) => T =
process.env.NODE_ENV === 'development'
? require('next/dist/compiled/@edge-runtime/primitives/structured-clone')
.structuredClone
: (v) => v
const resolvedMetadata = createDefaultMetadata()

let parentPromise = Promise.resolve(resolvedMetadata)

for (const item of metadataItems) {
Expand All @@ -263,13 +269,15 @@ export async function accumulateMetadata(

parentPromise = parentPromise.then((resolved) => {
return layerMetadataPromise.then((metadata) => {
resolved = deepClone(resolved)
merge(resolved, metadata, staticFilesMetadata, {
title: resolved.title?.template || null,
openGraph: resolved.openGraph?.title?.template || null,
twitter: resolved.twitter?.title?.template || null,
})

return resolved
return process.env.NODE_ENV === 'development'
? Object.freeze(resolved)
: resolved
})
})
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/metadata/app/params/[slug]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default function page(props) {

export async function generateMetadata(props, parent) {
const parentMetadata = await parent

/* mutating */
return {
...parentMetadata,
title: format(props),
Expand Down
33 changes: 32 additions & 1 deletion test/e2e/app-dir/metadata/metadata.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import { check, hasRedbox, getRedboxDescription } from 'next-test-utils'
import { BrowserInterface } from 'test/lib/browsers/base'

createNextDescribe(
Expand Down Expand Up @@ -317,6 +317,37 @@ createNextDescribe(
)
})

if (isNextDev) {
it('should freeze parent resolved metadata to avoid mutating in generateMetadata', async () => {
const pagePath = 'app/mutate/page.tsx'
const content = `export default function page(props) {
return <p>mutate</p>
}
export async function generateMetadata(props, parent) {
const parentMetadata = await parent
parentMetadata.x = 1
return {
...parentMetadata,
}
}`
await next.patchFile(pagePath, content)

const browser = await next.browser('/mutate')
await check(
async () => ((await hasRedbox(browser, true)) ? 'success' : 'fail'),
/success/
)
const error = await getRedboxDescription(browser)

await next.deleteFile(pagePath)

expect(error).toContain(
'Cannot add property x, object is not extensible'
)
})
}

it('should support synchronous generateMetadata export', async () => {
const browser = await next.browser('/basic/sync-generate-metadata')
expect(await getTitle(browser)).toBe('synchronous generateMetadata')
Expand Down

0 comments on commit 2b2c746

Please sign in to comment.