feat(fonts)!: local provider unification#15213
Conversation
🦋 Changeset detectedLatest commit: f806926 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 |
github-actions
bot
added
the
pkg: astro
…b.com/withastro/astro into feat/fonts-local-provider-unification
…b.com/withastro/astro into feat/fonts-local-provider-unification
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
There's a lot of code moved around, and I didn't have a chance to look all of it. If the tests pass (which are a lot), happy to merge it. Please update the RFC (didn't see it mentioned in the PR)
| name: "Custom", | ||
| cssVariable: "--font-custom", | ||
| - provider: "local", | ||
| + provider: fontProviders.local(), |
| // The index keeps track of encountered URLs. We can't use a regular for loop | ||
| // below because it may contain sources without urls, which would prevent preloading completely | ||
| let index = 0; | ||
| for (const source of font.src) { |
There was a problem hiding this comment.
Based on the comment, Can't we use a filter before this loop?
There was a problem hiding this comment.
We could but I think that would be a bit less efficient? Like require 2 loops instead of 1
|
@ematipico sorry again for the huge diff, I know it's not great. I had updated the RFC before requesting a review |
sarah11918
left a comment
sarah11918
left a comment
There was a problem hiding this comment.
I can't believe this is the LAST. EXPERIMENTAL. FONTS. PR. @florian-lefebvre 🥳
Nice work, just some very small comments from me here, and the docs PR is good to go with one tiny nit!
|
|
||
| Previously, there were 2 kinds of font providers: remote and local. | ||
|
|
||
| Font providers are now unified. If you are using the local provider, here is how you can update: |
There was a problem hiding this comment.
| Font providers are now unified. If you are using the local provider, here is how you can update: | |
| Font providers are now unified. If you are using the local provider, the process for configuring local fonts must be updated: |
| }] | ||
| } | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
| ``` | |
| ``` | |
| Once configured, there is no change to using local fonts in your project. However, you should inspect your deployed site to confirm that your new font configuration is being applied. | |
| See [the experimental Fonts API docs](https://docs.astro.build/en/reference/experimental-flags/fonts/) for more information. | |
| ````` |
|
|
||
| Exposes `root` on `FontProvider` `init()` context | ||
|
|
||
| When building a `FontProvider` for the experimental Fonts API, the `init()` method receives a `context`. This context now exposes a `root` URL, useful for resolving local files: |
There was a problem hiding this comment.
| When building a `FontProvider` for the experimental Fonts API, the `init()` method receives a `context`. This context now exposes a `root` URL, useful for resolving local files: | |
| When building a custom `FontProvider` for the experimental Fonts API, the `init()` method receives a `context`. This context now exposes a `root` URL, useful for resolving local files: |
I think this helps people quickly grok "Oh, I'm not building my own font provider... I can ignore!" (Assuming that's the intention)
| title: 'Font buffer not found', | ||
| message: (url: string) => | ||
| `No buffer was found for the \`"${url}"\` passed to the \`getFontBuffer()\` function.`, | ||
| `No buffer was found for the \`"${url}"\` url passed to the \`getFontBuffer()\` function.`, |
There was a problem hiding this comment.
| `No buffer was found for the \`"${url}"\` url passed to the \`getFontBuffer()\` function.`, | |
| `No buffer was found for the \`"${url}"\` URL passed to the \`getFontBuffer()\` function.`, |
If this is just regular paragraph text, then URL is upper cased. (If you want to describe an API's option, I'd use url inline code syntax.)
3 participants
Changes
Testing
Docs