-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Webfonts: remove unused font-face declarations #39332
Comments
After several iterations, I believe this issue is near the finish line. It's in good state but I'd really appreciate feedback. We have 3 PRs: #39361This PR changes how the internals of the #39327This changes how the Webfonts API works a bit by allowing webfont registration without immediately enqueueing. The change comes in the mindset, where instead of having to filter out unused webfonts, we're including the fonts we want to actually see in the front-end. This allows for more predictable behavior on which fonts are included in the output. It's more extensible and WP-developer friendly as well, since it follows common patterns defined by It does not maintain backward compatibility with how #39399This is the PR actually includes the registered webfonts in the output. It does so by scanning (and caching) content through all standardized post types in WordPress, including global styles, templates, template parts, posts, pages, and widgets. |
I think we need to clarify the use cases a bit to determine the best course of action. For example, a user should be able to register a set of fonts they like and see them as options in a UI like global styles, font pairs, etc. But only the fonts the user has assigned within their theme.json representation should be generating front-end enqueues. Does that seem reasonable? Are there other use cases missing? I'd say that the font-family selector of blocks should only be showing the set that comes from global styles, not every registered font. |
Hi @mtias, Yeah, all use-cases you described are covered. We are also aiming to let plugin developers use the API through the familiar |
Status Updates
|
Status UpdateSeparation between registration and enqueueingThe first version of the Webfonts API is a big monolith -- we're registering a font face and directly emitting the registering font family in which the font face belongs to the front-end, regardless if they're being used or not. That's why this issue exists in the first place: it quickly adds up in the size of the file that's sent to the front-end. The second version allows separation between registration (fonts that show up in the editor and can be picked) and enqueueing (fonts that actually end up being emitted in the front-end). It does so by exposing The The new approach lives here: #39559. Webfont scanningWebfonts picked in the editor end up as block metadata. On page load, the Its code lives here: #39593. Performance impact and cachingIn previous iterations of this issue, we were with a different mindset of scanning the group of blocks -- namely post types such as This introduced fragile, hard-to-read, and reason about solutions, both for the people writing (@jeyip and I, mostly) and reviewers. @creativecoder then came with similar concerns and we agreed that it was not good enough. That's when we decided to go the Instead of scanning (introducing new logic over template fetching and parsing) we're now hooking into a process that already happens (the "But Luis, wouldn't that greatly impact the server processing?" Not really. On our tests, it added about 20 ms on every page load in a very constrained Docker container, which looks like a totally viable tradeoff considering the simplicity of the new solution. You can compare:
It is much much simpler, and less fragile since Fonts from
|
I've been thinking a lot about our approach here while testing the related PRs over the past week+. Here are some thoughts on the implementation: Font enqueuing for block typography settings should happen at the block levelThe font family settings for blocks are provided by the block supports typography module (https://github.com/WordPress/gutenberg/blob/3739bd76e236ff6ed5431022dbd5684fc7f93ade/lib/block-supports/typography.php). I think that anything related to how font settings are structured, serialized/deserialized, etc should stay contained in that module, and helper functions added, if needed, for accessing settings. The fact that blocks are stored in various post types (posts, pages, template parts, etc) is incidental, and the font enqueuing logic shouldn't need to be aware of this. If we can handle enqueuing used fonts during block rendering, that seems best. Enqueuing block styles/scripts might be analogous, ideally assets that are needed are only enqueued when we've confirmed a block is being rendered on the page, and the rendering of the block itself should trigger that process. It looks like the newest approach in #39593 starts to address this, using the theme.json needs an option for enqueuing vs registering fontsAs we've seen from comments on the initial PRs (#39036 (comment)), there are good use cases for both registering and enqueuing fonts from theme.json. Sometimes you want that font-family to be there for use in the theme's stylesheet, or a specific block may be missing typography settings you want to apply manually with css. I think we'll need to add a property that forces enqueuing the font from theme.json when the theme author wants to guarantee that font is available, even if not used in a block setting or Global styles. The
|
I'm concerned about the some of the proposed changes in the Webfonts API as it's being shifted away from being source agnostic to being solely specific for only theme.json themes. The original intent of this API is for it to become a Core API that supports web fonts regardless of if they are registered by a theme.json powered theme, a legacy theme, or a plugin(s). The API IMO should not be aware of the theme.json or global styles settings or tasks. That specific knowledge and how-to is better encapsulated within the theme.json parser/handler, global styles handler, and/or a new service that preps before registration (sits in between). For example, if there are customizations from the global styles that need to be registered within the Webfonts API, the discovery work should be done elsewhere and then the array of webfonts arguments (its configuration) registered with the API. |
Awareness: Webfonts API will grow into more than only supporting local font file assets. It will have bundled remote font factory / foundry providers too such as Google Fonts. I raise this point for awareness as improvements made |
Thanks a lot for the comments, @hellofromtonya! Progress update: Register and enqueue webfonts by font-family #39559The PR is ready for review and ready to be merged after approval. Handle font face conflictsWhen registering through several providers, or really just different font faces for the same family, we're using its slug (derived from the font family name) as the aggregator. The problem is that with multiple providers, the same font face might get registered twice. To address that, we're thinking about wrapping the webfont array into a Scan content for font families and enqueue them #39593This PR will be reworked so we can move the logic from the Register font faces through
|
That is a valid concern, but at the same time, perhaps Jetpack should NOT be doing that?? Why does a plugin need to register 30 webfonts? Isn't there a better way for the plugin to do this? |
@aristath I think the use case is less about what Jetpack might be doing and more about having the ability for a user or theme to manage fonts in their libraries without incurring a penalty on the front-end. For example, a theme might have a couple styles with different font pairs, and it'd be a nice experience to be able to switch between the styles on the fly in the admin editor (all registered fonts accessible) while minimizing the ones that get enqueued on the front just to those that are actually used. Same if we allow users to upload font assets so they can manage their fonts and switch between them easily. It's not that disimilar to blocks registering their scripts and all getting enqueued in the editor but not necessarily on the front. |
Thank you for the extra context @mtias 👍 |
I think I'm fully caught up with the developments now and wanted to summarize a few things:
I think this is a pretty straightforward path. The main thing to clarify is that |
Right. Time for the action plan for ProblemWe want to enqueue font families referenced within The It is possible, and that's expected, to enqueue fonts by using the Proposed solutionAll the following snippets are inserted in Enqueue a font family{
"fontFamily": "Roboto",
"slug": "roboto",
"name": "Roboto"
}, This declaration would enqueue all font faces registered under the Enqueueing a subset of a font family{
"fontFamily": "Roboto",
"slug": "roboto",
"name": "Roboto",
"fontFaces": [
{
"fontFamily": "Roboto",
"fontStyle": "normal",
"fontStretch": "normal",
"fontWeight": "400"
}
]
} Specifying the All of the snippets above consider the font family being registered programmatically, through the Webfonts API. Registering and enqueueing font faces through a single providerAdding the {
"fontFamily": "Roboto",
"provider": "local",
"slug": "roboto",
"name": "Roboto",
"fontFaces": [
{
"fontFamily": "Roboto",
"fontStyle": "normal",
"fontStretch": "normal",
"fontWeight": "400",
"src": [ "file:./assets/fonts/Roboto-Regular.ttf" ]
}
]
} In this case, The Registering and enqueueing font faces through multiple providers{
"fontFamily": "Roboto",
"slug": "roboto",
"name": "Roboto",
"fontFaces": [
{
"provider": "local",
"fontFamily": "Roboto",
"fontStyle": "normal",
"fontStretch": "normal",
"fontWeight": "400",
"src": [ "file:./assets/fonts/Roboto-Regular.ttf" ]
},
{
"provider": "google-fonts",
"fontFamily": "Roboto",
"fontStyle": "bold",
"fontStretch": "normal",
"fontWeight": "700",
"src": [ "file:./assets/fonts/Roboto-Bold.ttf" ]
}
]
} Specifying QuestionsHow to differentiate between fonts that should be handled by the Webfonts API vs fonts that should notIt might be hard to differentiate between fonts that should be handled by the Webfonts API and fonts that are just system fonts. For example: {
"fontFamily": "\"DM Sans\", sans-serif",
"fontSlug": "dm-sans",
"slug": "heading-font",
"name": "Headings (DM Sans)",
"google": "family=DM+Sans:ital,wght@0,400;0,500;0,700;1,400;1,500;1,700"
},
{
"fontFamily": "Roboto",
"slug": "roboto",
"name": "Roboto"
}, Ignoring the My first idea was to add a property like But I also do not like implicit things. I think being explicit, especially in those cases, helps the developer understand what's missing when debugging or, when something fails, they'll know exactly what failed, or where the font is coming from! But, if we do want to be implicit and do not add the property... We can mark the font as external when running the Prefix Webfonts API attributes in
|
The Fonts API is now completely designed from when this issue was opened. It now extends from and reuses Core's Some of the linked work here are good enhancements. Given the changes in the API, each of the enhancements will need separate consideration. They no longer fit well into the scope of this issue. Rather, I think they fit better as separate enhancements within the Fonts API Ongoing Roadmap #41479. I'm closing this issue. Thank you everyone for your contributions 👏 Amazing work!! |
What problem does this address?
The initial Webfonts API adds
@font-face
declarations for every font-family that is registered, regardless if it is used in the css for the page, or not.With a few fonts, this is not too bad, but with additional font weights and styles, it can quickly add up. One example: Jetpack is adding a Google fonts module, and with ~30 Google fonts registered, there are over 1500 font-face declarations, adding well over 500 KB (uncompressed) to the page source.
What is your proposed solution?
Remove unused font-face declarations on each page to reduce the size of what is added to the page source.
Related tasks
The text was updated successfully, but these errors were encountered: