Skip to content

Commit

Permalink
Always collect static icons for all segments (#68712)
Browse files Browse the repository at this point in the history
### What

Collect static metadata icons from the most leaf node in the component
tree, and then add them into resolved metadata icons if there's no icons
presented in exported metadata

### Why

Previously we collected the icons from the last item from collected
metadata items as last segment in the tree. But it doesn't act like that
when there's parallel routes, so we collect the last presented static
metadata icons from the metadata item list and then merge into the
resolved metadata at the end.

Fixes #68650
  • Loading branch information
huozhi authored Aug 12, 2024
1 parent 5545de8 commit b80e0f6
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 21 deletions.
61 changes: 40 additions & 21 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { LoaderTree } from '../../server/lib/app-dir-module'
import type {
AbsoluteTemplateString,
IconDescriptor,
ResolvedIcons,
} from './types/metadata-types'
import type { ParsedUrlQuery } from 'querystring'
import type { StaticMetadata } from './types/icons'
Expand Down Expand Up @@ -49,6 +50,8 @@ import { ResolveMetadataSpan } from '../../server/lib/trace/constants'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/segment'
import * as Log from '../../build/output/log'

type StaticIcons = Pick<ResolvedIcons, 'icon' | 'apple'>

type MetadataResolver = (
parent: ResolvingMetadata
) => Metadata | Promise<Metadata>
Expand Down Expand Up @@ -99,27 +102,18 @@ function mergeStaticMetadata(
staticFilesMetadata: StaticMetadata,
metadataContext: MetadataContext,
titleTemplates: TitleTemplates,
isLastSegment: boolean
leafSegmentStaticIcons: StaticIcons
) {
if (!staticFilesMetadata) return
const { icon, apple, openGraph, twitter, manifest } = staticFilesMetadata

// Only pick up the static metadata if the current level is the last segment
if (isLastSegment) {
// file based metadata is specified and current level metadata icons is not specified
if (target.icons) {
if (icon) {
target.icons.icon.unshift(...icon)
}
if (apple) {
target.icons.apple.unshift(...apple)
}
} else if (icon || apple) {
target.icons = {
icon: icon || [],
apple: apple || [],
}
}
// Keep updating the static icons in the most leaf node

if (icon) {
leafSegmentStaticIcons.icon = icon
}
if (apple) {
leafSegmentStaticIcons.apple = apple
}

// file based metadata is specified and current level metadata twitter.images is not specified
Expand Down Expand Up @@ -158,15 +152,15 @@ function mergeMetadata({
titleTemplates,
metadataContext,
buildState,
isLastSegment,
leafSegmentStaticIcons,
}: {
source: Metadata | null
target: ResolvedMetadata
staticFilesMetadata: StaticMetadata
titleTemplates: TitleTemplates
metadataContext: MetadataContext
buildState: BuildState
isLastSegment: boolean
leafSegmentStaticIcons: StaticIcons
}): void {
// If there's override metadata, prefer it otherwise fallback to the default metadata.
const metadataBase =
Expand Down Expand Up @@ -290,7 +284,7 @@ function mergeMetadata({
staticFilesMetadata,
metadataContext,
titleTemplates,
isLastSegment
leafSegmentStaticIcons
)
}

Expand Down Expand Up @@ -774,6 +768,13 @@ export async function accumulateMetadata(
}

let favicon

// Collect the static icons in the most leaf node,
// since we don't collect all the static metadata icons in the parent segments.
const leafSegmentStaticIcons = {
icon: [],
apple: [],
}
for (let i = 0; i < metadataItems.length; i++) {
const staticFilesMetadata = metadataItems[i][1]

Expand All @@ -800,7 +801,7 @@ export async function accumulateMetadata(
staticFilesMetadata,
titleTemplates,
buildState,
isLastSegment: i === metadataItems.length - 1,
leafSegmentStaticIcons,
})

// If the layout is the same layer with page, skip the leaf layout and leaf page
Expand All @@ -814,6 +815,24 @@ export async function accumulateMetadata(
}
}

if (
leafSegmentStaticIcons.icon.length > 0 ||
leafSegmentStaticIcons.apple.length > 0
) {
if (!resolvedMetadata.icons) {
resolvedMetadata.icons = {
icon: [],
apple: [],
}
if (leafSegmentStaticIcons.icon.length > 0) {
resolvedMetadata.icons.icon.unshift(...leafSegmentStaticIcons.icon)
}
if (leafSegmentStaticIcons.apple.length > 0) {
resolvedMetadata.icons.apple.unshift(...leafSegmentStaticIcons.apple)
}
}
}

// Only log warnings if there are any, and only once after the metadata resolving process is finished
if (buildState.warnings.size > 0) {
for (const warning of buildState.warnings) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Modal() {
return <p>modal</p>
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
14 changes: 14 additions & 0 deletions test/e2e/app-dir/metadata-icons-parallel-routes/app/icon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions test/e2e/app-dir/metadata-icons-parallel-routes/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default function Root({ children, modal }) {
return (
<html>
<body>
{modal}
{children}
</body>
</html>
)
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>page</p>
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata-icons-parallel-routes/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>page</p>
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { nextTestSetup } from 'e2e-utils'

describe('app-dir - metadata-icons-parallel-routes', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should present favicon with other icons when parallel routes are presented', async () => {
const $ = await next.render$('/')
expect($('link[type="image/x-icon"]').length).toBe(1)
expect($('link[type="image/svg+xml"]').length).toBe(1)
expect($('link[rel="apple-touch-icon"]').length).toBe(1)
})

it('should override parent icon when both static icon presented', async () => {
const $ = await next.render$('/nested')
expect($('link[type="image/x-icon"]').length).toBe(1)
expect($('link[rel="icon"][type="image/png"]').length).toBe(1)
})

it('should inherit parent apple icon when child does not present but parent contain static apple icon', async () => {
const $ = await next.render$('/nested')
expect($('link[rel="apple-touch-icon"][type="image/png"]').length).toBe(1)
})
})

0 comments on commit b80e0f6

Please sign in to comment.