Skip to content

Commit

Permalink
Fix icons metadata missing from layout (#52673)
Browse files Browse the repository at this point in the history
### What

This was an issue introduced in #52416 when we check the images and icons property while merging with static file conventions. Where we should check if the properties are present but we only checked if they value is falsy. So when you're using `icons` with array value it breaks.

Now we properly check all the possible case of `metadata.icons` value, so then decide if we are using the static file convention or the icons property of defined metadata.

Also add few more tests cases to cover icons resolving.
  • Loading branch information
huozhi committed Jul 14, 2023
1 parent 3763f4f commit d10f105
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 2 deletions.
124 changes: 124 additions & 0 deletions packages/next/src/lib/metadata/resolve-metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,130 @@ describe('accumulateMetadata', () => {
})
})

describe('icon', () => {
it('should resolve icons.icon correctly', async () => {
// array icons
expect(
await accumulateMetadata([
[
{
icons: [
{
url: 'favicon-light.png',
rel: 'icon',
media: '(prefers-color-scheme: light)',
},
{
url: 'favicon-dark.png',
rel: 'icon',
media: '(prefers-color-scheme: dark)',
},
],
},
null,
],
])
).toMatchObject({
icons: {
icon: [
{
url: 'favicon-light.png',
rel: 'icon',
media: '(prefers-color-scheme: light)',
},
{
url: 'favicon-dark.png',
rel: 'icon',
media: '(prefers-color-scheme: dark)',
},
],
},
})

// string icons
expect(
await accumulateMetadata([
[
{
icons: 'favicon-light.png',
},
null,
],
])
).toMatchObject({
icons: {
icon: [
{
url: 'favicon-light.png',
rel: 'icon',
},
],
},
})

// icon.icons array
expect(
await accumulateMetadata([
[
{
icons: {
icon: [
{
url: 'favicon-light.png',
},
{
url: 'favicon-dark.png',
},
],
},
},
null,
],
])
).toMatchObject({
icons: {
icon: [
{
url: 'favicon-light.png',
},
{
url: 'favicon-dark.png',
},
],
},
})
})

it('should resolve icons.apple', async () => {
expect(
await accumulateMetadata([
[
{
icons: {
apple: [
{
url: 'apple-touch-icon-light.png',
media: '(prefers-color-scheme: light)',
},
],
},
},
null,
],
])
).toMatchObject({
icons: {
apple: [
{
url: 'apple-touch-icon-light.png',
media: '(prefers-color-scheme: light)',
},
],
},
})
})
})

describe('itunes', () => {
it('should resolve relative url starting with ./ with pathname for itunes.appArgument', async () => {
const metadataItems: MetadataItems = [
Expand Down
23 changes: 21 additions & 2 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,25 @@ type TitleTemplates = {
openGraph: string | null
}

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 mergeStaticMetadata(
source: Metadata | null,
target: ResolvedMetadata,
Expand All @@ -61,8 +80,8 @@ function mergeStaticMetadata(
const { icon, apple, openGraph, twitter, manifest } = staticFilesMetadata
// file based metadata is specified and current level metadata icons is not specified
if (
(icon && !source?.icons && !source?.icons?.hasOwnProperty('icon')) ||
(apple && !source?.icons?.hasOwnProperty('apple'))
(icon && !hasIconsProperty(source?.icons, 'icon')) ||
(apple && !hasIconsProperty(source?.icons, 'apple'))
) {
target.icons = {
icon: icon || [],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export default function layout({ children }) {
return children
}

export const metadata = {
icons: [
{
url: 'favicon-light.png',
rel: 'icon',
media: '(prefers-color-scheme: light)',
},
{
url: 'favicon-dark.png',
rel: 'icon',
media: '(prefers-color-scheme: dark)',
},
],
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function page() {
return 'icons-from-layout'
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/metadata/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,18 @@ createNextDescribe(
})
})

it('should merge icons from layout if no static icons files are specified', async () => {
const browser = await next.browser('/icons/descriptor/from-layout')
const matchDom = createDomMatcher(browser)

await matchDom('link', 'href="favicon-light.png"', {
media: '(prefers-color-scheme: light)',
})
await matchDom('link', 'href="favicon-dark.png"', {
media: '(prefers-color-scheme: dark)',
})
})

it('should not hoist meta[itemProp] to head', async () => {
const $ = await next.render$('/')
expect($('head meta[itemProp]').length).toBe(0)
Expand Down

0 comments on commit d10f105

Please sign in to comment.