Skip to content

Commit

Permalink
Fix favicon merging with customized icons (#67982)
Browse files Browse the repository at this point in the history
Support merging the static metadata file conventions `favicon.ico` with
other customized metadata icon from the module export.

`favicon.ico` should be displayed everywhere as it's the icon
representative of the website for all pages. Any customized `icon` in
metadata export `export const metadata` or `export function
generateMetadata()` shouldn't override the `favicon.ico` file
convention.

Fixes #55767

---------

Co-authored-by: hrmny <8845940+ForsakenHarmony@users.noreply.github.com>
  • Loading branch information
huozhi and ForsakenHarmony committed Aug 14, 2024
1 parent d46b852 commit 2c27829
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 52 deletions.
13 changes: 1 addition & 12 deletions crates/next-core/src/app_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,17 +871,6 @@ async fn directory_tree_to_loader_tree(
.then_some(components.page)
.flatten()
{
// When resolving metadata with corresponding module
// (https://github.com/vercel/next.js/blob/aa1ee5995cdd92cc9a2236ce4b6aa2b67c9d32b2/packages/next/src/lib/metadata/resolve-metadata.ts#L340)
// layout takes precedence over page (https://github.com/vercel/next.js/blob/aa1ee5995cdd92cc9a2236ce4b6aa2b67c9d32b2/packages/next/src/server/lib/app-dir-module.ts#L22)
// If the component have layout and page both, do not attach same metadata to
// the page.
let metadata = if components.layout.is_some() {
Default::default()
} else {
components.metadata.clone()
};

tree.parallel_routes.insert(
"children".into(),
LoaderTree {
Expand All @@ -890,7 +879,7 @@ async fn directory_tree_to_loader_tree(
parallel_routes: IndexMap::new(),
components: Components {
page: Some(page),
metadata,
metadata: components.metadata,
..Default::default()
}
.cell(),
Expand Down
6 changes: 5 additions & 1 deletion crates/next-core/src/loader_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ impl LoaderTreeBuilder {
metadata: &Metadata,
global_metadata: Option<&GlobalMetadata>,
) -> Result<()> {
if metadata.is_empty() {
if metadata.is_empty()
&& global_metadata
.map(|global| global.is_empty())
.unwrap_or_default()
{
return Ok(());
}
let Metadata {
Expand Down
116 changes: 82 additions & 34 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ import type { OpenGraph } from './types/opengraph-types'
import type { ComponentsType } from '../../build/webpack/loaders/next-app-loader'
import type { MetadataContext } from './types/resolvers'
import type { LoaderTree } from '../../server/lib/app-dir-module'
import type { AbsoluteTemplateString } from './types/metadata-types'
import type {
AbsoluteTemplateString,
IconDescriptor,
} from './types/metadata-types'
import type { ParsedUrlQuery } from 'querystring'
import type { StaticMetadata } from './types/icons'

import {
createDefaultMetadata,
Expand All @@ -37,15 +41,14 @@ import {
resolveThemeColor,
resolveVerification,
resolveItunes,
resolveFacebook,
} from './resolvers/resolve-basics'
import { resolveIcons } from './resolvers/resolve-icons'
import { getTracer } from '../../server/lib/trace/tracer'
import { ResolveMetadataSpan } from '../../server/lib/trace/constants'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/segment'
import * as Log from '../../build/output/log'

type StaticMetadata = Awaited<ReturnType<typeof resolveStaticMetadata>>

type MetadataResolver = (
parent: ResolvingMetadata
) => Metadata | Promise<Metadata>
Expand All @@ -56,7 +59,7 @@ type ViewportResolver = (
export type MetadataItems = [
Metadata | MetadataResolver | null,
StaticMetadata,
Viewport | ViewportResolver | null
Viewport | ViewportResolver | null,
][]

type TitleTemplates = {
Expand All @@ -77,49 +80,54 @@ type PageProps = {
searchParams: { [key: string]: any }
}

function hasIconsProperty(
icons: Metadata['icons'],
prop: 'icon' | 'apple'
): boolean {
if (!icons) return false
if (prop === 'icon') {
// Detect if icons.icon will be presented, icons array and icons string will all be merged into icons.icon
return !!(
typeof icons === 'string' ||
icons instanceof URL ||
Array.isArray(icons) ||
(prop in icons && icons[prop])
)
} else {
// Detect if icons.apple will be presented, only icons.apple will be merged into icons.apple
return !!(typeof icons === 'object' && prop in icons && icons[prop])
function isFavicon(icon: IconDescriptor | undefined): boolean {
if (!icon) {
return false
}

// turbopack appends a hash to all images
return (
(icon.url === '/favicon.ico' ||
icon.url.toString().startsWith('/favicon.ico?')) &&
icon.type === 'image/x-icon'
)
}

function mergeStaticMetadata(
source: Metadata | null,
target: ResolvedMetadata,
staticFilesMetadata: StaticMetadata,
metadataContext: MetadataContext,
titleTemplates: TitleTemplates
titleTemplates: TitleTemplates,
isLastSegment: boolean
) {
if (!staticFilesMetadata) return
const { icon, apple, openGraph, twitter, manifest } = staticFilesMetadata
// file based metadata is specified and current level metadata icons is not specified
if (
(icon && !hasIconsProperty(source?.icons, 'icon')) ||
(apple && !hasIconsProperty(source?.icons, 'apple'))
) {
target.icons = {
icon: icon || [],
apple: apple || [],

// 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 || [],
}
}
}

// file based metadata is specified and current level metadata twitter.images is not specified
if (twitter && !source?.twitter?.hasOwnProperty('images')) {
const resolvedTwitter = resolveTwitter(
{ ...target.twitter, images: twitter } as Twitter,
target.metadataBase,
metadataContext,
titleTemplates.twitter
)
target.twitter = resolvedTwitter
Expand Down Expand Up @@ -150,13 +158,15 @@ function mergeMetadata({
titleTemplates,
metadataContext,
buildState,
isLastSegment,
}: {
source: Metadata | null
target: ResolvedMetadata
staticFilesMetadata: StaticMetadata
titleTemplates: TitleTemplates
metadataContext: MetadataContext
buildState: BuildState
isLastSegment: boolean
}): void {
// If there's override metadata, prefer it otherwise fallback to the default metadata.
const metadataBase =
Expand Down Expand Up @@ -192,10 +202,15 @@ function mergeMetadata({
target.twitter = resolveTwitter(
source.twitter,
metadataBase,
metadataContext,
titleTemplates.twitter
)
break
}
case 'facebook':
target.facebook = resolveFacebook(source.facebook)
break

case 'verification':
target.verification = resolveVerification(source.verification)
break
Expand Down Expand Up @@ -274,7 +289,8 @@ function mergeMetadata({
target,
staticFilesMetadata,
metadataContext,
titleTemplates
titleTemplates,
isLastSegment
)
}

Expand Down Expand Up @@ -376,7 +392,10 @@ async function collectStaticImagesFiles(
: undefined
}

async function resolveStaticMetadata(components: ComponentsType, props: any) {
async function resolveStaticMetadata(
components: ComponentsType,
props: any
): Promise<StaticMetadata> {
const { metadata } = components
if (!metadata) return null

Expand Down Expand Up @@ -566,7 +585,9 @@ function inheritFromMetadata(
const commonOgKeys = ['title', 'description', 'images'] as const
function postProcessMetadata(
metadata: ResolvedMetadata,
titleTemplates: TitleTemplates
favicon: any,
titleTemplates: TitleTemplates,
metadataContext: MetadataContext
): ResolvedMetadata {
const { openGraph, twitter } = metadata

Expand Down Expand Up @@ -599,6 +620,7 @@ function postProcessMetadata(
const partialTwitter = resolveTwitter(
autoFillProps,
metadata.metadataBase,
metadataContext,
titleTemplates.twitter
)
if (metadata.twitter) {
Expand All @@ -620,6 +642,17 @@ function postProcessMetadata(
inheritFromMetadata(openGraph, metadata)
inheritFromMetadata(twitter, metadata)

if (favicon) {
if (!metadata.icons) {
metadata.icons = {
icon: [],
apple: [],
}
}

metadata.icons.icon.unshift(favicon)
}

return metadata
}

Expand Down Expand Up @@ -672,7 +705,7 @@ async function getMetadataFromExport<Data, ResolvedData>(
// Only preload at the beginning when resolves are empty
if (!dynamicMetadataResolvers.length) {
for (let j = currentIndex; j < metadataItems.length; j++) {
const preloadMetadataExport = getPreloadMetadataExport(metadataItems[j]) // metadataItems[j][0]
const preloadMetadataExport = getPreloadMetadataExport(metadataItems[j])
// call each `generateMetadata function concurrently and stash their resolver
if (typeof preloadMetadataExport === 'function') {
collectMetadataExportPreloading<Data, ResolvedData>(
Expand Down Expand Up @@ -739,9 +772,18 @@ export async function accumulateMetadata(
const buildState = {
warnings: new Set<string>(),
}

let favicon
for (let i = 0; i < metadataItems.length; i++) {
const staticFilesMetadata = metadataItems[i][1]

// Treat favicon as special case, it should be the first icon in the list
// i <= 1 represents root layout, and if current page is also at root
if (i <= 1 && isFavicon(staticFilesMetadata?.icon?.[0])) {
const iconMod = staticFilesMetadata?.icon?.shift()
if (i === 0) favicon = iconMod
}

const metadata = await getMetadataFromExport<Metadata, ResolvedMetadata>(
(metadataItem) => metadataItem[0],
dynamicMetadataResolvers,
Expand All @@ -758,6 +800,7 @@ export async function accumulateMetadata(
staticFilesMetadata,
titleTemplates,
buildState,
isLastSegment: i === metadataItems.length - 1,
})

// If the layout is the same layer with page, skip the leaf layout and leaf page
Expand All @@ -778,7 +821,12 @@ export async function accumulateMetadata(
}
}

return postProcessMetadata(resolvedMetadata, titleTemplates)
return postProcessMetadata(
resolvedMetadata,
favicon,
titleTemplates,
metadataContext
)
}

export async function accumulateViewport(
Expand Down
7 changes: 7 additions & 0 deletions packages/next/src/lib/metadata/types/icons.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type StaticMetadata = {
icon: any[] | undefined
apple: any[] | undefined
openGraph: any[] | undefined
twitter: any[] | undefined
manifest: string | undefined
} | null
Binary file added test/e2e/app-dir/metadata-icons/app/favicon.ico
Binary file not shown.
20 changes: 20 additions & 0 deletions test/e2e/app-dir/metadata-icons/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { ReactNode } from 'react'

export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}

export const metadata = {
icons: {
shortcut: '/shortcut-icon.png',
apple: '/apple-icon.png',
other: {
rel: 'apple-touch-icon-precomposed',
url: '/apple-touch-icon-precomposed.png',
},
},
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/metadata-icons/app/nested/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export default function Page() {
return <p>hello world</p>
}

export const metadata = {
icons: {
shortcut: '/shortcut-icon-nested.png',
apple: '/apple-icon-nested.png',
other: {
rel: 'apple-touch-icon-precomposed-nested',
url: '/apple-touch-icon-precomposed-nested.png',
},
},
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata-icons/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>hello world</p>
}
31 changes: 31 additions & 0 deletions test/e2e/app-dir/metadata-icons/metadata-icons.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { nextTestSetup } from 'e2e-utils'

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

it('should only have 1 favicon link in root page', async () => {
const $ = await next.render$('/')
expect($('link[href^="/favicon.ico"]').length).toBe(1)
})

it('should only have 1 favicon link in nested page', async () => {
const $ = await next.render$('/nested')
expect($('link[href^="/favicon.ico"]').length).toBe(1)
})

it('should render custom icons along with favicon in root page', async () => {
const $ = await next.render$('/')
expect($('link[rel="shortcut icon"]').attr('href')).toBe(
'/shortcut-icon.png'
)
})

it('should render custom icons along with favicon in nested page', async () => {
const $ = await next.render$('/nested')
expect($('link[rel="shortcut icon"]').attr('href')).toBe(
'/shortcut-icon-nested.png'
)
})
})
Loading

0 comments on commit 2c27829

Please sign in to comment.