inject font preload links

[Rich-Harris]

fixes #4039.

I think we need to figure out some details before merging this though:

• do we put this behind an option? probably, since otherwise you might end up loading fonts that aren't even used on a given page (but are nonetheless imported by CSS)
• is it a config option, or a resolve option? resolve is more granular, though possibly unnecessarily so
• do we want to have the ability to preload other assets (images and such)? should there be some way to specify which assets are subject to preloading?

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 20c6df1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benmccann]

I'm surprised this was all that was needed. When I had looked at this awhile ago the font info wasn't in the build and the only way to get it was setting ssrManifest, but I guess something must have changed

Anyway, in terms of the option, I think you might want something more than a boolean. Like a function that takes the page URL and a list of the font paths and allows you to filter to decide which to preload. In that case it'd have to be done via resolve. You could also make it applied to all preloaded assets and pass the asset type as well and then the user would have control over the images as well if desired - though images need to be preloaded far less often

[benmccann]

Vite plans to add options for disabling, filtering, and transform of preload URLs after v3. I wonder if we should just keep this on the backburner for awhile and see what v4 brings in terms of allowing the user to configure preload URLs so that we don't duplicate Vite's options.

[Rich-Harris]

v4 could be a long ways off, no?

[benmccann]

I think the idea is to do it around the end of this year. They want to do a new version for Rollup 3 support

[benmccann]

But actually, Vite 4 is probably irrelevant because adding a new option is backwards compatible. The actual update I saw this morning on Vite Discord was "we'll work on this after v3 release" and my mind somehow filled that in with Vite 4

[Rich-Harris]

What would it even look like though, given that we're not using transformIndexHtml? It feels like something that falls within the SSR framework's responsibilities. Either way, font waterfalls are one of the things that are most harmful to real and perceived performance, so it feels like something it'd be good to get in sooner

[benmccann]

We would need to be ultimately responsible for rendering the preload links. We could potentially use Vite's options though for filtering the links. I have no objection to adding this PR as is. I think the trickier thing would be whether we want to add an API for filtering preload links when Vite will add its own.

[benmccann]

Here's a PR for Vite's new option: vitejs/vite#9938

[dummdidumm]

@benmccann that PR was since merged. What does this mean for the state of this PR? How would we use those APIs?

[benmccann]

I haven't looked into it very much yet, so don't know a whole lot beyond what's in the docs: https://vitejs.dev/config/build-options.html#build-modulepreload. It looks to be called via Vite's preload helper when there's a dynamic import, so we should probably use it to ensure dynamic imports are handled.

I think there'd be two options:

• we create our own option but then provide a modulePreload function which is computed from the value specified to our option and prevent users from providing their own modulePreload filter
• we have users use the modulePreload filter and we execute this function ourselves when server rendering a page. This introduces less API surface area, but ties us more closely to Vite and would make it harder to support multiple bundlers

[dummdidumm]

Updated this to master.

I think I prefer the "enhance handle with a method that is called for every asset/font/js/css" route:

export function handle({ event, resolve }) {
  return resolve(event, { preload: ({ type, url }) => {
    // this would be the default; or we decide to also not preload fonts;
    // or we decide to add fonts to assets and remove that special type
    if (type !== 'asset') { // type: 'font' | 'css' | 'js' | 'asset'
      return true;
    }
  });

Routing every asset through there could be done later in a non-breaking way if we either treat asset as a special type with which for now the method is never called; or if we decide "not font type, just an asset type" then we can still make clear in the docs that this will potentially be called with more asset types in the future and you should use a regex on the url to narrow it to what you want.

[Rich-Harris]

Yeah, I think that API makes sense, and I'd probably be in favour of defaulting to preloading everything (we'd need to see what impact it has on different sites before committing). I think pathname probably makes more sense than url.

[dummdidumm]

After reading a bit I think it would be a mistake to preload all assets by default. In fact, things like Next's imagine component do the opposite and load images on demand only. Preloading all this by default would probably hurt core web vitals for the average page.

I think the best way forward is to

• have type: js/css/font/asset
• only repload the first three by default
• note in the docs that we don't preload assets at all currently, but may do so in the future if the need arises (this way it's non-breaking)