Skip to content

Commit fff183c

Browse files
Font optimization bug fix (vercel#24162)
## Bug - [x] Related issues linked using `fixes vercel#23896` - [x] Integration tests added Fixes vercel#23896
1 parent fc53879 commit fff183c

File tree

4 files changed

+101
-43
lines changed

4 files changed

+101
-43
lines changed

packages/next/next-server/lib/post-process.ts

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,9 @@ type postProcessOptions = {
1313
type renderOptions = {
1414
getFontDefinition?: (url: string) => string
1515
}
16-
17-
type PostProcessData = {
18-
preloads: {
19-
images: Array<string>
20-
}
21-
}
22-
2316
interface PostProcessMiddleware {
24-
inspect: (
25-
originalDom: HTMLElement,
26-
data: PostProcessData,
27-
options: renderOptions
28-
) => void
29-
mutate: (
30-
markup: string,
31-
data: PostProcessData,
32-
options: renderOptions
33-
) => Promise<string>
17+
inspect: (originalDom: HTMLElement, options: renderOptions) => any
18+
mutate: (markup: string, data: any, options: renderOptions) => Promise<string>
3419
}
3520

3621
type middlewareSignature = {
@@ -58,18 +43,13 @@ async function processHTML(
5843
if (!middlewareRegistry[0]) {
5944
return html
6045
}
61-
const postProcessData: PostProcessData = {
62-
preloads: {
63-
images: [],
64-
},
65-
}
6646
const root: HTMLElement = parse(html)
6747
let document = html
6848
// Calls the middleware, with some instrumentation and logging
6949
async function callMiddleWare(middleware: PostProcessMiddleware) {
7050
// let timer = Date.now()
71-
middleware.inspect(root, postProcessData, data)
72-
document = await middleware.mutate(document, postProcessData, data)
51+
const inspectData = middleware.inspect(root, data)
52+
document = await middleware.mutate(document, inspectData, data)
7353
// timer = Date.now() - timer
7454
// if (timer > MIDDLEWARE_TIME_BUDGET) {
7555
// TODO: Identify a correct upper limit for the postprocess step
@@ -89,15 +69,11 @@ async function processHTML(
8969
}
9070

9171
class FontOptimizerMiddleware implements PostProcessMiddleware {
92-
fontDefinitions: (string | undefined)[][] = []
93-
inspect(
94-
originalDom: HTMLElement,
95-
_data: PostProcessData,
96-
options: renderOptions
97-
) {
72+
inspect(originalDom: HTMLElement, options: renderOptions) {
9873
if (!options.getFontDefinition) {
9974
return
10075
}
76+
const fontDefinitions: (string | undefined)[][] = []
10177
// collecting all the requested font definitions
10278
originalDom
10379
.querySelectorAll('link')
@@ -115,30 +91,35 @@ class FontOptimizerMiddleware implements PostProcessMiddleware {
11591
const nonce = element.getAttribute('nonce')
11692

11793
if (url) {
118-
this.fontDefinitions.push([url, nonce])
94+
fontDefinitions.push([url, nonce])
11995
}
12096
})
97+
98+
return fontDefinitions
12199
}
122100
mutate = async (
123101
markup: string,
124-
_data: PostProcessData,
102+
fontDefinitions: string[][],
125103
options: renderOptions
126104
) => {
127105
let result = markup
128106
if (!options.getFontDefinition) {
129107
return markup
130108
}
131-
for (const key in this.fontDefinitions) {
132-
const [url, nonce] = this.fontDefinitions[key]
109+
110+
fontDefinitions.forEach((fontDef) => {
111+
const [url, nonce] = fontDef
133112
const fallBackLinkTag = `<link rel="stylesheet" href="${url}"/>`
134113
if (
135114
result.indexOf(`<style data-href="${url}">`) > -1 ||
136115
result.indexOf(fallBackLinkTag) > -1
137116
) {
138117
// The font is already optimized and probably the response is cached
139-
continue
118+
return
140119
}
141-
const fontContent = options.getFontDefinition(url as string)
120+
const fontContent = options.getFontDefinition
121+
? options.getFontDefinition(url as string)
122+
: null
142123
if (!fontContent) {
143124
/**
144125
* In case of unreachable font definitions, fallback to default link tag.
@@ -151,13 +132,15 @@ class FontOptimizerMiddleware implements PostProcessMiddleware {
151132
`<style data-href="${url}"${nonceStr}>${fontContent}</style></head>`
152133
)
153134
}
154-
}
135+
})
136+
155137
return result
156138
}
157139
}
158140

159141
class ImageOptimizerMiddleware implements PostProcessMiddleware {
160-
inspect(originalDom: HTMLElement, _data: PostProcessData) {
142+
inspect(originalDom: HTMLElement) {
143+
const imgPreloads = []
161144
const imgElements = originalDom.querySelectorAll('img')
162145
let eligibleImages: Array<HTMLElement> = []
163146
for (let i = 0; i < imgElements.length; i++) {
@@ -169,18 +152,18 @@ class ImageOptimizerMiddleware implements PostProcessMiddleware {
169152
}
170153
}
171154

172-
_data.preloads.images = []
173-
174155
for (const imgEl of eligibleImages) {
175156
const src = imgEl.getAttribute('src')
176157
if (src) {
177-
_data.preloads.images.push(src)
158+
imgPreloads.push(src)
178159
}
179160
}
161+
162+
return imgPreloads
180163
}
181-
mutate = async (markup: string, _data: PostProcessData) => {
164+
mutate = async (markup: string, imgPreloads: string[]) => {
182165
let result = markup
183-
let imagePreloadTags = _data.preloads.images
166+
let imagePreloadTags = imgPreloads
184167
.filter((imgHref) => !preloadTagAlreadyExists(markup, imgHref))
185168
.reduce(
186169
(acc, imgHref) =>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import Head from 'next/head'
2+
import Link from 'next/link'
3+
4+
const WithFont = () => {
5+
return (
6+
<>
7+
<Head>
8+
<link
9+
href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;700;900&display=swap"
10+
rel="stylesheet"
11+
/>
12+
</Head>
13+
14+
<div>
15+
Page with custom fonts
16+
<br />
17+
<br />
18+
<Link href="/without-font">Without font</Link>
19+
</div>
20+
</>
21+
)
22+
}
23+
24+
export const getServerSideProps = async () => {
25+
return {
26+
props: {},
27+
}
28+
}
29+
30+
export default WithFont
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import Head from 'next/head'
2+
import Link from 'next/link'
3+
4+
const WithoutFont = () => {
5+
return (
6+
<>
7+
<Head>
8+
<meta name="robots" content="noindex, nofollow" />
9+
</Head>
10+
<div>
11+
Page without custom fonts
12+
<br />
13+
<br />
14+
<Link href="/with-font">With font</Link>
15+
</div>
16+
</>
17+
)
18+
}
19+
20+
export const getServerSideProps = async () => {
21+
return {
22+
props: {},
23+
}
24+
}
25+
26+
export default WithoutFont

test/integration/font-optimization/test/index.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,25 @@ function runTests() {
102102
)
103103
})
104104

105+
it('should only inline included fonts per page', async () => {
106+
const html = await renderViaHTTP(appPort, '/with-font')
107+
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
108+
expect(html).toContain(
109+
'<link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;700;900&amp;display=swap"/>'
110+
)
111+
expect(html).toMatch(
112+
/<style data-href="https:\/\/fonts\.googleapis\.com\/css2\?family=Roboto:wght@400;700;900&display=swap">.*<\/style>/
113+
)
114+
115+
const htmlWithoutFont = await renderViaHTTP(appPort, '/without-font')
116+
expect(htmlWithoutFont).not.toContain(
117+
'<link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;700;900&amp;display=swap"/>'
118+
)
119+
expect(htmlWithoutFont).not.toMatch(
120+
/<style data-href="https:\/\/fonts\.googleapis\.com\/css2\?family=Roboto:wght@400;700;900&display=swap">.*<\/style>/
121+
)
122+
})
123+
105124
it.skip('should minify the css', async () => {
106125
const snapshotJson = JSON.parse(
107126
await fs.readFile(join(__dirname, 'manifest-snapshot.json'), {

0 commit comments

Comments
 (0)