-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: custom fonts with preloading #209
Changes from 8 commits
9ba2ff7
600586d
5a9cad9
6705816
dd5afce
42e0685
1856f0f
f43745e
c70708f
374ae94
e1f0cff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// example use of custom fonts. Replace these with your own fonts: | ||
import archivo400url from '@fontsource/archivo/files/archivo-latin-400-normal.woff2?url'; | ||
import archivo600url from '@fontsource/archivo/files/archivo-latin-600-normal.woff2?url'; | ||
|
||
// urls above only resolve correctly in production; imports below are for development: | ||
import '@fontsource/archivo/latin-400.css'; | ||
import '@fontsource/archivo/latin-600.css'; | ||
|
||
export type Font = { | ||
family: string; | ||
weight: number; | ||
style: string; | ||
woff2Url: string; | ||
} | ||
|
||
export const fontFamilyArchivo = 'Archivo, sans-serif'; | ||
|
||
export const fonts: Font[] = [ | ||
{ | ||
family: 'Archivo', | ||
weight: 400, | ||
style: 'normal', | ||
woff2Url: archivo400url, | ||
}, | ||
{ | ||
family: 'Archivo', | ||
weight: 600, | ||
style: 'normal', | ||
woff2Url: archivo600url, | ||
}, | ||
]; | ||
|
||
export const getFontCss = (font: Font) => { | ||
return /* css */` | ||
@font-face { | ||
font-family: '${font.family}'; | ||
font-style: ${font.style}; | ||
font-weight: ${font.weight}; | ||
src: url('${font.woff2Url}') format('woff2'); | ||
} | ||
`; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,22 @@ | ||
--- | ||
jbmoelker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import { datocmsAssetsOrigin, datocmsGraphqlOrigin } from '@lib/datocms'; | ||
import { fonts, getFontCss } from '@assets/fonts'; | ||
|
||
const preConnectOrigins = [datocmsAssetsOrigin, datocmsGraphqlOrigin]; | ||
const woff2urls = fonts.map((font) => font.woff2Url); | ||
const criticalCss = fonts.map((font) => getFontCss(font)).join('\n'); | ||
--- | ||
|
||
{preConnectOrigins.map((origin) => <link rel="preconnect" href={origin} />)} | ||
{ | ||
woff2urls.map((url) => ( | ||
<link | ||
rel="preload" | ||
as="font" | ||
type="font/woff2" | ||
href={url} | ||
crossorigin="anonymous" | ||
/> | ||
)) | ||
} | ||
<style set:text={criticalCss}></style> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think this serves any purpose since font files will only start to download when CSS is parsed and the browser knows wether there are characters on the screen that need a custom font. This is why font file downloads start late and we preload them with resource hints. Just a font face declaration will not cause the font to be downloaded. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or am I missing something? If not, I think this PR can be "cleaned up" a lot. You could even remove the fonts.ts file all together since you only need the imports for the font files. So in perfhead something like this would be sufficient:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You would still need to define the CSS declarations somewhere. It would make sense to me to do that in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checked again, and this does actually work. The reason being that both the critical CSS and the global core styles are inlined in the default layout. You can verify this on the the deploy preview. If you check the page source code you find both the font declarations (critical CSS) and the usage of the font family (might be easiest to search for |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import { describe, expect, test, } from 'vitest'; | ||
import { renderToFragment } from '@lib/renderer'; | ||
import PerfHead from './PerfHead.astro'; | ||
|
||
|
||
describe('PerfHead', async () => { | ||
const fragment = await renderToFragment(PerfHead, { props: {} }); | ||
|
||
test('pre-connects to origins to speed up following requests', () => { | ||
const links = fragment.querySelectorAll('link[rel="preconnect"]'); | ||
expect(links.length).toBeGreaterThanOrEqual(1); | ||
links.forEach((link) => { | ||
// expect link's href to be a valid absolute URL: | ||
const url = new URL(String(link.getAttribute('href'))); | ||
expect(url.protocol).toMatch(/https?/); | ||
}); | ||
}); | ||
|
||
test('preloads font files for faster font loading and page rendering', () => { | ||
const links = fragment.querySelectorAll('link[rel="preload"][as="font"]'); | ||
expect(links.length).toBeGreaterThanOrEqual(1); | ||
links.forEach((link) => { | ||
expect(link.getAttribute('type')).toBe('font/woff2'); | ||
expect(link.getAttribute('href')).toMatch(/\.woff2$/); | ||
expect(link.getAttribute('crossorigin')).toBe('anonymous'); | ||
}); | ||
}); | ||
|
||
test('inlines font declarations as critical CSS', () => { | ||
const styleText = fragment.querySelector('style')?.textContent ?? ''; | ||
const fontFaceDeclarations = styleText.match(/@font-face\s*{[^}]*}/g) ?? []; | ||
expect(fontFaceDeclarations.length).toBeGreaterThanOrEqual(1); | ||
fontFaceDeclarations.forEach((declaration) => { | ||
// replace HTML entities with their character equivalents: | ||
const cleanedDeclaration = declaration.replace(/'/g, '\''); | ||
// expect font-face declaration to contain woff2 src url: | ||
expect(cleanedDeclaration).toMatch(/src\s*:\s*url\('.*\.woff2'\)\s*format\('woff2'\)/); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: import raw (
import archivo400css from '@fontsource/archivo/latin-400.css?raw';
) is not an option as it results in font urls starting with./files/...
that are not resolved. So instead we generate the font declaration ourselves with the url that is resolved correctly in production.Note 2: I've left out
font-display: swap;
as we don't have a fallback yet that doesn't cause a layout shift. See to do on Fontaine and font swap.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using fontaine seems a bit involved. Rather keep this PR small and merge this first, to add font swap as perf enhancement in a separate PR.