Skip to content

Commit

Permalink
Fix missing favicon when other icon exist (#48311)
Browse files Browse the repository at this point in the history
When collecting static icons we need both collect the one from layout
and page, but for root level route `/` we missed the `favicon.ico`
before so when other icon existed, the root page's collected icons will
cover root layout collected ones, which resulted into favicon missing

Fixes #48147
Closes NEXT-976
  • Loading branch information
huozhi committed Apr 12, 2023
1 parent 848e6fb commit 1d30045
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
6 changes: 3 additions & 3 deletions packages/next/src/build/webpack/loaders/metadata/discover.ts
Expand Up @@ -57,13 +57,13 @@ export async function createStaticMetadataFromRoute(
{
segment,
resolvePath,
isRootLayer,
isRootLayoutOrRootPage,
loaderContext,
pageExtensions,
}: {
segment: string
resolvePath: (pathname: string) => Promise<string>
isRootLayer: boolean
isRootLayoutOrRootPage: boolean
loaderContext: webpack.LoaderContext<any>
pageExtensions: string[]
}
Expand Down Expand Up @@ -122,7 +122,7 @@ export async function createStaticMetadataFromRoute(
collectIconModuleIfExists('apple'),
collectIconModuleIfExists('openGraph'),
collectIconModuleIfExists('twitter'),
isRootLayer && collectIconModuleIfExists('favicon'),
isRootLayoutOrRootPage && collectIconModuleIfExists('favicon'),
])

return hasStaticMetadataFiles ? staticImagesMetadata : null
Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/build/webpack/loaders/next-app-loader.ts
Expand Up @@ -237,6 +237,7 @@ async function createTreeCodeFromPath(
// Existing tree are the children of the current segment
const props: Record<string, string> = {}
const isRootLayer = segments.length === 0
const isRootLayoutOrRootPage = segments.length <= 1

// We need to resolve all parallel routes in this level.
const parallelSegments: [key: string, segment: string | string[]][] = []
Expand All @@ -256,7 +257,7 @@ async function createTreeCodeFromPath(
metadata = await createStaticMetadataFromRoute(resolvedRouteDir, {
segment: segmentPath,
resolvePath,
isRootLayer,
isRootLayoutOrRootPage,
loaderContext,
pageExtensions,
})
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/app-dir/metadata/app/icon.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 10 additions & 7 deletions test/e2e/app-dir/metadata/metadata.test.ts
Expand Up @@ -547,15 +547,18 @@ createNextDescribe(

it('should support root level of favicon.ico', async () => {
let $ = await next.render$('/')
let $icon = $('link[rel="icon"]')
expect($icon.attr('href')).toBe('/favicon.ico')
expect($icon.attr('type')).toBe('image/x-icon')
expect($icon.attr('sizes')).toBe('any')
const favIcon = $('link[rel="icon"]')
expect(favIcon.attr('href')).toBe('/favicon.ico')
expect(favIcon.attr('type')).toBe('image/x-icon')
expect(favIcon.attr('sizes')).toBe('any')

const iconSvg = $('link[rel="icon"][type="image/svg+xml"]')
expect(iconSvg.attr('href')).toBe('/icon.svg?90699bff34adba1f')

$ = await next.render$('/basic')
$icon = $('link[rel="icon"]')
expect($icon.attr('href')).toBe('/favicon.ico')
expect($icon.attr('sizes')).toBe('any')
const icon = $('link[rel="icon"]')
expect(icon.attr('href')).toBe('/favicon.ico')
expect(icon.attr('sizes')).toBe('any')
})
})

Expand Down

0 comments on commit 1d30045

Please sign in to comment.