From 6ad91722954049ffb5e23435f2b3fb4ba94d7cc2 Mon Sep 17 00:00:00 2001 From: Janicklas Ralph Date: Wed, 12 May 2021 04:39:26 -0700 Subject: [PATCH] Font optimization bug fix (#24968) --- packages/next/client/head-manager.ts | 12 ++++ packages/next/next-server/lib/head.tsx | 4 ++ .../build-output/test/index.test.js | 4 +- .../fixtures/with-google/pages/index.js | 12 +++- .../fixtures/with-google/pages/with-font.js | 2 +- .../fixtures/with-google/server.js | 29 +++++---- .../fixtures/with-typekit/pages/index.js | 12 +++- .../fixtures/with-typekit/pages/with-font.js | 2 +- .../fixtures/with-typekit/server.js | 29 +++++---- .../font-optimization/test/index.test.js | 59 +++++++++++++++---- 10 files changed, 127 insertions(+), 38 deletions(-) diff --git a/packages/next/client/head-manager.ts b/packages/next/client/head-manager.ts index 99d7cee807a53..b6d872b358571 100644 --- a/packages/next/client/head-manager.ts +++ b/packages/next/client/head-manager.ts @@ -100,6 +100,18 @@ export default function initHeadManager(): { const tags: Record = {} 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 diff --git a/packages/next/next-server/lib/head.tsx b/packages/next/next-server/lib/head.tsx index f035ef5797485..64d98ac6a7998 100644 --- a/packages/next/next-server/lib/head.tsx +++ b/packages/next/next-server/lib/head.tsx @@ -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) } } diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 55e800f4c31a5..9e2064a1c708d 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -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')) { @@ -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) diff --git a/test/integration/font-optimization/fixtures/with-google/pages/index.js b/test/integration/font-optimization/fixtures/with-google/pages/index.js index 39cf2d1f7b376..e32705671ee44 100644 --- a/test/integration/font-optimization/fixtures/with-google/pages/index.js +++ b/test/integration/font-optimization/fixtures/with-google/pages/index.js @@ -1,7 +1,17 @@ import React from 'react' +import Link from 'next/link' const Page = () => { - return
Hi!
+ return ( +
+ Hi! +
+
+ + With font + +
+ ) } export default Page diff --git a/test/integration/font-optimization/fixtures/with-google/pages/with-font.js b/test/integration/font-optimization/fixtures/with-google/pages/with-font.js index b9f8875b371fc..ef6b67a94fa12 100644 --- a/test/integration/font-optimization/fixtures/with-google/pages/with-font.js +++ b/test/integration/font-optimization/fixtures/with-google/pages/with-font.js @@ -11,7 +11,7 @@ const WithFont = () => { /> -
+
Page with custom fonts

diff --git a/test/integration/font-optimization/fixtures/with-google/server.js b/test/integration/font-optimization/fixtures/with-google/server.js index 6a98fa3d30806..64921f037eac9 100644 --- a/test/integration/font-optimization/fixtures/with-google/server.js +++ b/test/integration/font-optimization/fixtures/with-google/server.js @@ -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( diff --git a/test/integration/font-optimization/fixtures/with-typekit/pages/index.js b/test/integration/font-optimization/fixtures/with-typekit/pages/index.js index 39cf2d1f7b376..e32705671ee44 100644 --- a/test/integration/font-optimization/fixtures/with-typekit/pages/index.js +++ b/test/integration/font-optimization/fixtures/with-typekit/pages/index.js @@ -1,7 +1,17 @@ import React from 'react' +import Link from 'next/link' const Page = () => { - return
Hi!
+ return ( +
+ Hi! +
+
+ + With font + +
+ ) } export default Page diff --git a/test/integration/font-optimization/fixtures/with-typekit/pages/with-font.js b/test/integration/font-optimization/fixtures/with-typekit/pages/with-font.js index b1a1fa4011d49..06c062283639f 100644 --- a/test/integration/font-optimization/fixtures/with-typekit/pages/with-font.js +++ b/test/integration/font-optimization/fixtures/with-typekit/pages/with-font.js @@ -8,7 +8,7 @@ const WithFont = () => { -
+
Page with custom fonts

diff --git a/test/integration/font-optimization/fixtures/with-typekit/server.js b/test/integration/font-optimization/fixtures/with-typekit/server.js index 6a98fa3d30806..64921f037eac9 100644 --- a/test/integration/font-optimization/fixtures/with-typekit/server.js +++ b/test/integration/font-optimization/fixtures/with-typekit/server.js @@ -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( diff --git a/test/integration/font-optimization/test/index.test.js b/test/integration/font-optimization/test/index.test.js index 76b5d4b5d9c4a..aa4a5f775ebec 100644 --- a/test/integration/font-optimization/test/index.test.js +++ b/test/integration/font-optimization/test/index.test.js @@ -10,7 +10,9 @@ import { nextBuild, renderViaHTTP, initNextServerScript, + waitFor, } from 'next-test-utils' +import webdriver from 'next-webdriver' jest.setTimeout(1000 * 60 * 2) @@ -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', () => { @@ -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( - `` - ) + 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( - `` - ) + 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( - `` - ) + 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(``) + 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'), { @@ -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() })