Skip to content

Commit

Permalink
Font optimization bug fix (#24968)
Browse files Browse the repository at this point in the history
  • Loading branch information
janicklas-ralph authored May 12, 2021
1 parent b859c5b commit 6ad9172
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 38 deletions.
12 changes: 12 additions & 0 deletions packages/next/client/head-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@ export default function initHeadManager(): {
const tags: Record<string, JSX.Element[]> = {}

head.forEach((h) => {
if (
// If the font tag is loaded only on client navigation
// it won't be inlined. In this case revert to the original behavior
h.type === 'link' &&
h.props['data-optimized-fonts'] &&
!document.querySelector(
`style[data-href="${h.props['data-href']}"]`
)
) {
h.props.href = h.props['data-href']
h.props['data-href'] = undefined
}
const components = tags[h.type] || []
components.push(h)
tags[h.type] = components
Expand Down
4 changes: 4 additions & 0 deletions packages/next/next-server/lib/head.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ function reduceComponents(
const newProps = { ...(c.props || {}) }
newProps['data-href'] = newProps['href']
newProps['href'] = undefined

// Add this attribute to make it easy to identify optimized tags
newProps['data-optimized-fonts'] = true

return React.cloneElement(c, newProps)
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('Build Output', () => {
expect(parseFloat(err404FirstLoad)).toBeCloseTo(66.3, 0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(63.3, 1)
expect(parseFloat(sharedByAll)).toBeCloseTo(63.4, 1)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand All @@ -122,7 +122,7 @@ describe('Build Output', () => {
expect(parseFloat(webpackSize) - 952).toBeLessThanOrEqual(0)
expect(webpackSize.endsWith(' B')).toBe(true)

expect(parseFloat(mainSize) - 19.3).toBeLessThanOrEqual(0)
expect(parseFloat(mainSize) - 19.4).toBeLessThanOrEqual(0)
expect(mainSize.endsWith('kB')).toBe(true)

expect(parseFloat(frameworkSize) - 42.1).toBeLessThanOrEqual(0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import React from 'react'
import Link from 'next/link'

const Page = () => {
return <div>Hi!</div>
return (
<div>
Hi!
<br />
<br />
<Link href="/with-font">
<a id="with-font">With font</a>
</Link>
</div>
)
}

export default Page
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const WithFont = () => {
/>
</Head>

<div>
<div id="with-font-container">
Page with custom fonts
<br />
<br />
Expand Down
29 changes: 19 additions & 10 deletions test/integration/font-optimization/fixtures/with-google/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,27 @@ const server = http.createServer(async (req, res) => {
}

if (pathname.startsWith('/_next/static/')) {
res.write(
fs.readFileSync(
path.join(
__dirname,
'./.next/static/',
decodeURI(pathname.slice('/_next/static/'.length))
),
'utf8'
let prom = Promise.resolve()
if (pathname.endsWith('.css')) {
prom = new Promise((resolve) => setTimeout(resolve, 20000))
}
prom.then(() => {
res.write(
fs.readFileSync(
path.join(
__dirname,
'./.next/static/',
decodeURI(pathname.slice('/_next/static/'.length))
),
'utf8'
)
)
)
return res.end()
return res.end()
})
} else {
if (pathname === '') {
pathname = '/index'
}
const ext = isDataReq ? 'json' : 'html'
if (
fs.existsSync(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import React from 'react'
import Link from 'next/link'

const Page = () => {
return <div>Hi!</div>
return (
<div>
Hi!
<br />
<br />
<Link href="/with-font">
<a id="with-font">With font</a>
</Link>
</div>
)
}

export default Page
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const WithFont = () => {
<link href="https://use.typekit.net/ucs7mcf.css" rel="stylesheet" />
</Head>

<div>
<div id="with-font-container">
Page with custom fonts
<br />
<br />
Expand Down
29 changes: 19 additions & 10 deletions test/integration/font-optimization/fixtures/with-typekit/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,27 @@ const server = http.createServer(async (req, res) => {
}

if (pathname.startsWith('/_next/static/')) {
res.write(
fs.readFileSync(
path.join(
__dirname,
'./.next/static/',
decodeURI(pathname.slice('/_next/static/'.length))
),
'utf8'
let prom = Promise.resolve()
if (pathname.endsWith('.css')) {
prom = new Promise((resolve) => setTimeout(resolve, 20000))
}
prom.then(() => {
res.write(
fs.readFileSync(
path.join(
__dirname,
'./.next/static/',
decodeURI(pathname.slice('/_next/static/'.length))
),
'utf8'
)
)
)
return res.end()
return res.end()
})
} else {
if (pathname === '') {
pathname = '/index'
}
const ext = isDataReq ? 'json' : 'html'
if (
fs.existsSync(
Expand Down
59 changes: 47 additions & 12 deletions test/integration/font-optimization/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import {
nextBuild,
renderViaHTTP,
initNextServerScript,
waitFor,
} from 'next-test-utils'
import webdriver from 'next-webdriver'

jest.setTimeout(1000 * 60 * 2)

Expand All @@ -33,7 +35,7 @@ const startServerlessEmulator = async (dir, port) => {
{ ...process.env },
{ PORT: port, BUILD_ID: await getBuildId(dir) }
)
return initNextServerScript(scriptPath, /ready on/i, env)
return initNextServerScript(scriptPath, /ready on/i, env, false, {})
}

describe('Font Optimization', () => {
Expand Down Expand Up @@ -122,38 +124,70 @@ describe('Font Optimization', () => {

it(`should inline the ${property} fonts for static pages`, async () => {
const html = await renderViaHTTP(appPort, '/index')
const $ = cheerio.load(html)
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
expect(html).toContain(
`<link rel="stylesheet" data-href="${staticFont}"/>`
)
expect(
$(`link[rel=stylesheet][data-href="${staticFont}"]`).length
).toBe(1)
expect(html).toMatch(staticPattern)
})

it(`should inline the ${property} fonts for static pages with Next/Head`, async () => {
const html = await renderViaHTTP(appPort, '/static-head')
const $ = cheerio.load(html)
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
expect(html).toContain(
`<link rel="stylesheet" data-href="${staticHeadFont}"/>`
)
expect(
$(`link[rel=stylesheet][data-href="${staticHeadFont}"]`).length
).toBe(1)
expect(html).toMatch(staticHeadPattern)
})

it(`should inline the ${property} fonts for SSR pages`, async () => {
const html = await renderViaHTTP(appPort, '/stars')
const $ = cheerio.load(html)
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
expect(html).toContain(
`<link rel="stylesheet" data-href="${starsFont}"/>`
)
expect(
$(`link[rel=stylesheet][data-href="${starsFont}"]`).length
).toBe(1)
expect(html).toMatch(starsPattern)
})

it('should skip this optimization for AMP pages', async () => {
const html = await renderViaHTTP(appPort, '/amp')
const $ = cheerio.load(html)
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
expect(html).toContain(`<link rel="stylesheet" href="${staticFont}">`)
expect($(`link[rel=stylesheet][href="${staticFont}"]`).length).toBe(1)
expect(html).not.toMatch(staticPattern)
})

it('should work for fonts loaded on navigation', async () => {
let browser
try {
browser = await webdriver(appPort, '/')
await waitFor(1000)

const baseFont = await browser.elementByCss(
`style[data-href="${staticFont}"]`
)
expect(baseFont).toBeDefined()

await browser.waitForElementByCss('#with-font')
await browser.click('#with-font')

await browser.waitForElementByCss('#with-font-container')
const pageFontCss = await browser.elementsByCss(
`style[data-href="${withFont}"]`
)
expect(pageFontCss.length).toBe(0)
const pageFont = await browser.elementByCss(
`link[href="${withFont}"]`
)
expect(pageFont).toBeDefined()
} finally {
if (browser) await browser.close()
}
})

it('should minify the css', async () => {
const snapshotJson = JSON.parse(
await fs.readFile(join(appDir, 'manifest-snapshot.json'), {
Expand Down Expand Up @@ -225,12 +259,13 @@ describe('Font Optimization', () => {
)
await nextBuild(appDir)
appPort = await findPort()
await startServerlessEmulator(appDir, appPort)
app = await startServerlessEmulator(appDir, appPort)
builtServerPagesDir = join(appDir, '.next', 'serverless')
builtPage = (file) => join(builtServerPagesDir, file)
})
afterAll(async () => {
await fs.remove(nextConfig)
await killApp(app)
})
runTests()
})
Expand Down

0 comments on commit 6ad9172

Please sign in to comment.