Dedupe next/font preload tags (#46354)
Just like https://github.com/vercel/next.js/pull/44938 did for CSS tags, this makes sure `app-render` will only render one preload tag per font file by keeping track of the rendered files in a set while building the component tree. Tested locally with react, react-dom and react-server-dom-webpack with version `18.3.0-next-4fcc9184a-20230217`. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
This commit is contained in:
parent
cf1637421c
commit
18232f6e75
12 changed files with 138 additions and 5 deletions
|
@ -638,7 +638,8 @@ function getPreloadedFontFilesInlineLinkTags(
|
|||
serverCSSManifest: FlightCSSManifest,
|
||||
fontLoaderManifest: FontLoaderManifest | undefined,
|
||||
serverCSSForEntries: string[],
|
||||
filePath?: string
|
||||
filePath: string | undefined,
|
||||
injectedFontPreloadTags: Set<string>
|
||||
): string[] | null {
|
||||
if (!fontLoaderManifest || !filePath) {
|
||||
return null
|
||||
|
@ -652,7 +653,6 @@ function getPreloadedFontFilesInlineLinkTags(
|
|||
}
|
||||
|
||||
const fontFiles = new Set<string>()
|
||||
// If we find an entry in the manifest but it's empty, add a preconnect tag
|
||||
let foundFontUsage = false
|
||||
|
||||
for (const css of layoutOrPageCss) {
|
||||
|
@ -662,13 +662,21 @@ function getPreloadedFontFilesInlineLinkTags(
|
|||
if (preloadedFontFiles) {
|
||||
foundFontUsage = true
|
||||
for (const fontFile of preloadedFontFiles) {
|
||||
fontFiles.add(fontFile)
|
||||
if (!injectedFontPreloadTags.has(fontFile)) {
|
||||
fontFiles.add(fontFile)
|
||||
injectedFontPreloadTags.add(fontFile)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!foundFontUsage) {
|
||||
// If we find an entry in the manifest but it's empty, add a preconnect tag by returning null.
|
||||
// Only render a preconnect tag if we previously didn't preload any fonts.
|
||||
if (
|
||||
!foundFontUsage ||
|
||||
(fontFiles.size === 0 && injectedFontPreloadTags.size > 0)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
|
||||
|
@ -1083,6 +1091,7 @@ export async function renderToHTMLOrFlight(
|
|||
firstItem,
|
||||
rootLayoutIncluded,
|
||||
injectedCSS,
|
||||
injectedFontPreloadTags,
|
||||
}: {
|
||||
createSegmentPath: CreateSegmentPath
|
||||
loaderTree: LoaderTree
|
||||
|
@ -1090,6 +1099,7 @@ export async function renderToHTMLOrFlight(
|
|||
rootLayoutIncluded: boolean
|
||||
firstItem?: boolean
|
||||
injectedCSS: Set<string>
|
||||
injectedFontPreloadTags: Set<string>
|
||||
}): Promise<{ Component: React.ComponentType }> => {
|
||||
const [segment, parallelRoutes, components] = tree
|
||||
const {
|
||||
|
@ -1114,13 +1124,17 @@ export async function renderToHTMLOrFlight(
|
|||
)
|
||||
: []
|
||||
|
||||
const injectedFontPreloadTagsWithCurrentLayout = new Set(
|
||||
injectedFontPreloadTags
|
||||
)
|
||||
const preloadedFontFiles = layoutOrPagePath
|
||||
? getPreloadedFontFilesInlineLinkTags(
|
||||
serverComponentManifest,
|
||||
serverCSSManifest!,
|
||||
fontLoaderManifest,
|
||||
serverCSSForEntries,
|
||||
layoutOrPagePath
|
||||
layoutOrPagePath,
|
||||
injectedFontPreloadTagsWithCurrentLayout
|
||||
)
|
||||
: []
|
||||
|
||||
|
@ -1322,6 +1336,7 @@ export async function renderToHTMLOrFlight(
|
|||
parentParams: currentParams,
|
||||
rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove,
|
||||
injectedCSS: injectedCSSWithCurrentLayout,
|
||||
injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
|
||||
})
|
||||
|
||||
const childProp: ChildProp = {
|
||||
|
@ -1483,6 +1498,7 @@ export async function renderToHTMLOrFlight(
|
|||
parentRendered,
|
||||
rscPayloadHead,
|
||||
injectedCSS,
|
||||
injectedFontPreloadTags,
|
||||
rootLayoutIncluded,
|
||||
}: {
|
||||
createSegmentPath: CreateSegmentPath
|
||||
|
@ -1493,6 +1509,7 @@ export async function renderToHTMLOrFlight(
|
|||
parentRendered?: boolean
|
||||
rscPayloadHead: React.ReactNode
|
||||
injectedCSS: Set<string>
|
||||
injectedFontPreloadTags: Set<string>
|
||||
rootLayoutIncluded: boolean
|
||||
}): Promise<FlightDataPath> => {
|
||||
const [segment, parallelRoutes, components] = loaderTreeToFilter
|
||||
|
@ -1558,6 +1575,7 @@ export async function renderToHTMLOrFlight(
|
|||
parentParams: currentParams,
|
||||
firstItem: isFirst,
|
||||
injectedCSS,
|
||||
injectedFontPreloadTags,
|
||||
// This is intentionally not "rootLayoutIncludedAtThisLevelOrAbove" as createComponentTree starts at the current level and does a check for "rootLayoutAtThisLevel" too.
|
||||
rootLayoutIncluded: rootLayoutIncluded,
|
||||
}
|
||||
|
@ -1574,6 +1592,9 @@ export async function renderToHTMLOrFlight(
|
|||
// the result consistent.
|
||||
const layoutPath = layout?.[1]
|
||||
const injectedCSSWithCurrentLayout = new Set(injectedCSS)
|
||||
const injectedFontPreloadTagsWithCurrentLayout = new Set(
|
||||
injectedFontPreloadTags
|
||||
)
|
||||
if (layoutPath) {
|
||||
getCssInlinedLinkTags(
|
||||
serverComponentManifest,
|
||||
|
@ -1583,6 +1604,14 @@ export async function renderToHTMLOrFlight(
|
|||
injectedCSSWithCurrentLayout,
|
||||
true
|
||||
)
|
||||
getPreloadedFontFilesInlineLinkTags(
|
||||
serverComponentManifest,
|
||||
serverCSSManifest!,
|
||||
fontLoaderManifest,
|
||||
serverCSSForEntries,
|
||||
layoutPath,
|
||||
injectedFontPreloadTagsWithCurrentLayout
|
||||
)
|
||||
}
|
||||
|
||||
// Walk through all parallel routes.
|
||||
|
@ -1605,6 +1634,7 @@ export async function renderToHTMLOrFlight(
|
|||
isFirst: false,
|
||||
rscPayloadHead,
|
||||
injectedCSS: injectedCSSWithCurrentLayout,
|
||||
injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
|
||||
rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove,
|
||||
})
|
||||
|
||||
|
@ -1641,6 +1671,7 @@ export async function renderToHTMLOrFlight(
|
|||
</>
|
||||
),
|
||||
injectedCSS: new Set(),
|
||||
injectedFontPreloadTags: new Set(),
|
||||
rootLayoutIncluded: false,
|
||||
})
|
||||
).slice(1),
|
||||
|
@ -1738,6 +1769,7 @@ export async function renderToHTMLOrFlight(
|
|||
parentParams: {},
|
||||
firstItem: true,
|
||||
injectedCSS: new Set(),
|
||||
injectedFontPreloadTags: new Set(),
|
||||
rootLayoutIncluded: false,
|
||||
})
|
||||
|
||||
|
|
3
test/e2e/app-dir/next-font/app-old/navigation/font.js
Normal file
3
test/e2e/app-dir/next-font/app-old/navigation/font.js
Normal file
|
@ -0,0 +1,3 @@
|
|||
import localFont from '@next/font/local'
|
||||
|
||||
export const font = localFont({ src: './font.woff2' })
|
1
test/e2e/app-dir/next-font/app-old/navigation/font.woff2
Normal file
1
test/e2e/app-dir/next-font/app-old/navigation/font.woff2
Normal file
|
@ -0,0 +1 @@
|
|||
font.woff2
|
12
test/e2e/app-dir/next-font/app-old/navigation/layout.js
Normal file
12
test/e2e/app-dir/next-font/app-old/navigation/layout.js
Normal file
|
@ -0,0 +1,12 @@
|
|||
import { font } from './font'
|
||||
|
||||
export default function RootLayout({ children }) {
|
||||
return (
|
||||
<html lang="en">
|
||||
<body>
|
||||
<p className={font.className}>LAYOUT1</p>
|
||||
{children}
|
||||
</body>
|
||||
</html>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,9 @@
|
|||
import { font } from '../font'
|
||||
|
||||
export default function Page() {
|
||||
return (
|
||||
<p id="page-with-same-font" className={font.className}>
|
||||
Page with same font
|
||||
</p>
|
||||
)
|
||||
}
|
11
test/e2e/app-dir/next-font/app-old/navigation/page.js
Normal file
11
test/e2e/app-dir/next-font/app-old/navigation/page.js
Normal file
|
@ -0,0 +1,11 @@
|
|||
import Link from 'next/link'
|
||||
|
||||
export default function Page() {
|
||||
return (
|
||||
<>
|
||||
<Link href="/navigation/page-with-same-font">
|
||||
Go to page with same font
|
||||
</Link>
|
||||
</>
|
||||
)
|
||||
}
|
3
test/e2e/app-dir/next-font/app/navigation/font.js
Normal file
3
test/e2e/app-dir/next-font/app/navigation/font.js
Normal file
|
@ -0,0 +1,3 @@
|
|||
import localFont from 'next/font/local'
|
||||
|
||||
export const font = localFont({ src: './font.woff2' })
|
1
test/e2e/app-dir/next-font/app/navigation/font.woff2
Normal file
1
test/e2e/app-dir/next-font/app/navigation/font.woff2
Normal file
|
@ -0,0 +1 @@
|
|||
font.woff2
|
12
test/e2e/app-dir/next-font/app/navigation/layout.js
Normal file
12
test/e2e/app-dir/next-font/app/navigation/layout.js
Normal file
|
@ -0,0 +1,12 @@
|
|||
import { font } from './font'
|
||||
|
||||
export default function RootLayout({ children }) {
|
||||
return (
|
||||
<html lang="en">
|
||||
<body>
|
||||
<p className={font.className}>LAYOUT1</p>
|
||||
{children}
|
||||
</body>
|
||||
</html>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,9 @@
|
|||
import { font } from '../font'
|
||||
|
||||
export default function Page() {
|
||||
return (
|
||||
<p id="page-with-same-font" className={font.className}>
|
||||
Page with same font
|
||||
</p>
|
||||
)
|
||||
}
|
11
test/e2e/app-dir/next-font/app/navigation/page.js
Normal file
11
test/e2e/app-dir/next-font/app/navigation/page.js
Normal file
|
@ -0,0 +1,11 @@
|
|||
import Link from 'next/link'
|
||||
|
||||
export default function Page() {
|
||||
return (
|
||||
<>
|
||||
<Link href="/navigation/page-with-same-font">
|
||||
Go to page with same font
|
||||
</Link>
|
||||
</>
|
||||
)
|
||||
}
|
|
@ -231,6 +231,7 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
|
|||
expect($('link[rel="preconnect"]').length).toBe(0)
|
||||
|
||||
// From root layout
|
||||
expect($('link[as="font"]').length).toBe(3)
|
||||
expect(getAttrs($('link[as="font"]'))).toEqual([
|
||||
{
|
||||
as: 'font',
|
||||
|
@ -379,6 +380,34 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
|
|||
})
|
||||
}
|
||||
|
||||
describe('navigation', () => {
|
||||
it('should not have duplicate preload tags on navigation', async () => {
|
||||
const browser = await next.browser('/navigation')
|
||||
|
||||
// Before navigation, root layout imports the font
|
||||
const preloadBeforeNavigation = await browser.elementsByCss(
|
||||
'link[as="font"]'
|
||||
)
|
||||
expect(preloadBeforeNavigation.length).toBe(1)
|
||||
expect(await preloadBeforeNavigation[0].getAttribute('href')).toBe(
|
||||
'/_next/static/media/c287665b44f047d4-s.p.woff2'
|
||||
)
|
||||
|
||||
// Navigate to a page that also imports that font
|
||||
await browser.elementByCss('a').click()
|
||||
await browser.waitForElementByCss('#page-with-same-font')
|
||||
|
||||
// After navigating
|
||||
const preloadAfterNavigation = await browser.elementsByCss(
|
||||
'link[as="font"]'
|
||||
)
|
||||
expect(preloadAfterNavigation.length).toBe(1)
|
||||
expect(await preloadAfterNavigation[0].getAttribute('href')).toBe(
|
||||
'/_next/static/media/c287665b44f047d4-s.p.woff2'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
if (isDev) {
|
||||
describe('Dev errors', () => {
|
||||
it('should recover on font loader error', async () => {
|
||||
|
|
Loading…
Reference in a new issue