feat(fonts): experimental_getFontFileURL()

[florian-lefebvre]

Changes

• Closes No programmatic API to load registered fonts at build time #16139
• Previous attempt: fix(fonts): getFontBuffer() consumes too much memory #15327
• Introduces a new experimental_getFontBuffer() utility to allow working with font buffers during prerendering
• The big thing is that it works thanks to HTTP requests. So during prerendering we have to start a temporary node server

Testing

Added a lot of unit tests, as well as few integration tests

Docs

• errors
• feat(fonts): experimental_getFontFileURL() docs#13729
• changeset

[changeset-bot]

🦋 Changeset detected

Latest commit: d5e6f45

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

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

[codspeed-hq]

Merging this PR will not alter performance

✅ 18 untouched benchmarks

---

_{Comparing feat/fonts-experimental-get-font-buffer (d5e6f45) with main (b2d8eb3)}

[ArmandPhilippot]

Docs-wise, this LGTM, including the docs PR! 🎉

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[ematipico]

The use of url and requestedUrl is unclear and needs some comments to clear things up

[matthewp]

What alternatives to the http server did you try?

[florian-lefebvre]

I don't remember exactly because that was a while ago but that was the most logical given the dev and ssr implementations. FYI we wouldn't need a distinct http server if prerendering always ran using HTTP with a vite preview server

[matthewp]

can we limit the server being run only if the user opts-in to this some how? Like could we have an option in font config for wanting to get the buffer?

[florian-lefebvre]

Currently the server is only created if there's a prerender environment and if there are fonts files that could be used.

I assume if you want to have thos opt-in you're thinking about some potential issues? Would love to know which.

And to answer your question directly, in its stable form I will not be in favor of have this behind a flag, it should just work. Since this is experimental, I'm okay with having experimental.prerenderFontBuffer or something. But yeah I'm curious to know what you have in mind

[florian-lefebvre]

@ematipico PTAL! Logic is safer now, we only allow a strict set of urls. Much better 👍

[matthewp]

If you need to extract the URL like this, what is the benefit of the experimental_getFontBuffer function vs. just calling fetch() yourself?

[ArmandPhilippot]

For this point specifically, the purpose of getFontBuffer() is to avoid:

  const data = import.meta.env.DEV
    ? await fetch(new URL(fontPath, context.url.origin)).then(async (res) => res.arrayBuffer())
    : await readFile(new URL(`.${fontPath}`, outDir));

Because, on the user side, fetch() would only work if you're in dev mode or if there is an existing live website. If your website is not live yet, the build fails and you have to read the file locally.

But, this example doesn't work for all use cases. Depending on the build output, users have to use outDir or build.client (with the Node adapter at least) to resolve the right path. The getFontBuffer() helper is meant to simplify that FWIU.

However, yeah, this doesn't solve all the "DX issues".
I mean fontData["--font-roboto"][0]?.src[0]?.url is not nice. That's said, this is hard to know what the user wants. With multiple fonts and/or a multiple weights you might end up with something like fontData["--font-inter"].find((font) => font.style === "normal" && font.weight === "400")?.src[0]?.url;.

Perhaps it's possible to add options to filter on behalf of the user? But getFontBuffer() then do two things and may require renaming.

[florian-lefebvre]

Yes again the problem this API is solving is access to font files during the build, which is one of the main usecases. And for this fetch() is the ideal solution

Having a great DX around how to access the URL to pass to getFontBuffer() is a different topic on its own I think. I would like this PR to be low-level and then we can think about improving the DX (this was discussed on Discord a few months back) in another PR. We're quite free to experiment with it while it's in experimental

[matthewp]

I'm hesitant for a couple of reasons:

1. Starting an HTTP server for each build that quickly gets destructed is expensive.
2. Us doing fetch() on behalf of the user opens us vulnerabilities. I think we've regretted APIs that did this in the past.
3. Doing a fetch can be slow and I don't think it's obvious from the user's perspective that it's going over network to get the font buffer. I could see this becoming a foot-gun.

Feel most strong about (2), somewhat about (3), and less-so about (1).

Not a hard-no on this, just marking this as request changes for now while we keep discussing.

[matthewp]

Instead of doing the fetch on behalf of the user, what about a helper function that made constructing the URL easier, like fetch(getFontBufferURL(fontData["--font-roboto"], Astro.url))

I think this helps my concerns because:

1. We're not burdened with needing to sanitize the input.
2. It's obvious to the user what is actually happening.

We would still have the temporary server during the build in this scenario.

[florian-lefebvre]

I'm hesitant for a couple of reasons:

1. Starting an HTTP server for each build that quickly gets destructed is expensive.
2. Us doing fetch() on behalf of the user opens us vulnerabilities. I think we've regretted APIs that did this in the past.
3. Doing a fetch can be slow and I don't think it's obvious from the user's perspective that it's going over network to get the font buffer. I could see this becoming a foot-gun.

Feel most strong about (2), somewhat about (3), and less-so about (1).

1. Not worried about this personally but I agree it's not ideal. I wish we could use configurePreviewServer() that likely requires changing how prerendering works (out of scope)
2. Yeah I agree there's always a risk. I'm pretty confident about the implementation tho but I get your point
3. I think doing this over the network is the most logical way but I agree. Let's see what you proposed

Instead of doing the fetch on behalf of the user, what about a helper function that made constructing the URL easier, like fetch(getFontBufferURL(fontData["--font-roboto"], Astro.url))

I think this helps my concerns because:

1. We're not burdened with needing to sanitize the input.
2. It's obvious to the user what is actually happening.

We would still have the temporary server during the build in this scenario.

I agree it's nice that the users own the fetch logic but I think it needs a few changes. IMO there are 2 usecases:

1. Mapping over all the data inside eg. fontData["--font-roboto"] and going through all URLs
2. Wanting a specific font file (weight/style combination basically)

The design I propose only addresses 1 intentionally, 2 would be added later.

Now looking at your proposal, I would do this:

• getFontBufferURL(fontData["--font-roboto"]?.src[0]?.url, Astro.url): still give the responsibility to the user to provide the url. I agree the DX is not ideal just yet, but ouf of scope of this PR
• Still have validation on the URL: better DX to let the user know they did use something not meant to be used here
• Keep the temporary node server

Wdyt?

[matthewp]

@florian-lefebvre I think we're saying the same thing, basically dropping the experimental_fontBuffer function and having this helper URL constructor instead, with the user still calling fetch() themselves, right? Assuming so, yeah that sounds good to me. And yes, keeping the temp http server.

[ArmandPhilippot]

Thanks, this looks good to me! I left a question for the errors (is "buffer" an oversight?)

[github-actions]

e18e dependency analysis

No dependency warnings found.