Skip to content
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

preload fonts by default #10596

Open
benmccann opened this issue Aug 21, 2023 · 4 comments
Open

preload fonts by default #10596

benmccann opened this issue Aug 21, 2023 · 4 comments
Labels
needs-decision Not sure if we want to do this yet, also design work needed

Comments

@benmccann
Copy link
Member

benmccann commented Aug 21, 2023

Describe the problem

According to #4963, @Rich-Harris said we don't preload fonts by default because they may be imported but not used on a page. Though I'm not sure that's a great reason for us to not do it by default as that seems like the user has coded things incorrectly in that case

Describe the proposed solution

Maybe Vite could issue a warning for fonts that are imported, but never used if we really care about detecting that

Alternatives considered

No response

Importance

nice to have

Additional Information

Font preloading sometimes doesn't work before Vite 5. Fixed in vitejs/vite#14297

@dummdidumm
Copy link
Member

We removed loading fonts by default in #7704 because there were too many false positives. Fonts in general can be very dynamic (fallback fonts, load a different font based on a unicode range) and as such just loading them upfront when detected anywhere on the page was turning out to be the wrong default. I'm against switching this, unless we can somehow find out which fonts are definitely loaded.

@benmccann
Copy link
Member Author

Hmm. I'd be curious if we can get some data around this. I'll close this for now I guess

@dummdidumm
Copy link
Member

Reopening so we can continue at some point to look into this, but taking this off the 2.0 milestone

@dummdidumm dummdidumm reopened this Dec 8, 2023
@dummdidumm dummdidumm removed this from the 2.0 milestone Dec 8, 2023
@eltigerchino eltigerchino added the needs-decision Not sure if we want to do this yet, also design work needed label Dec 19, 2023
@Rich-Harris
Copy link
Member

A case study: the (unreleased) new site, which inherited the following code from the site in this repo...

/** @type {import('@sveltejs/kit').Handle} */
export async function handle({ event, resolve }) {
  return await resolve(event, {
    preload: ({ type }) => type === 'js' || type === 'css' || type === 'font'
  });
}

...got a terrible Lighthouse score because LCP was delayed while the browser preloaded every single variant of every font referenced by CSS. When you use something like @fontsource, that means you're downloading .woff (not just .woff2) files for every alphabet, even if you never use cyrillic or whatever.

That's why this is a terrible default. In an ideal world we would have some way to automate the process of preloading only the font files that are needed for content above the fold on a given page, but that's a ways off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Not sure if we want to do this yet, also design work needed
Projects
None yet
Development

No branches or pull requests

4 participants