Improve loading method for block styles #28358
Conversation
|
Size Change: +1 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
aristath
added
[Feature] Full Site Editing
[Feature] Themes
|
It sounds like a great next step. I hope to look at implementation details somehow next week. |
aristath
changed the title
|
EDIT: No longer an issue, #28395 got merged |
aristath
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
|
EDIT: transient removed in a later refactor |
vindl
requested review from
youknowriad,
jeyip,
mattwiebe,
vindl,
Addison-Stavlo,
Copons and
david-szabo97
|
Very nice work @aristath! 👍 I've given this a test as well. Everything worked as advertised. I couldn't see any visual regressions either. Tested with:
As smoothly as this worked for me, I think approval of this PR would be better coming from someone who is more familiar with how this fits into the bigger picture with WordPress core and how it handles styles. |
aristath
requested review from
ajitbohra,
chrisvanpatten and
mkaz
as code owners
Trac ticket + patch: https://core.trac.wordpress.org/ticket/52620 |
|
Does this ticket in any way focus on how to enqueue scripts which need to attach to the blocks but are not needed if the block is not present? |
@megphillips91 I wrote up a way to use |
No, the PR focuses exclusively on improving the way stylesheets get loaded when they get loaded separately, which will happen in an FSE theme, or in a classic theme that has opted-in using the |
| * @param string $styles The styles to be minified. | ||
| * @return string | ||
| */ | ||
| function gutenberg_get_minified_styles( $styles ) { |
There was a problem hiding this comment.
There's a bug in this function: You can easily see it by checking the "button" block in the "emptytheme". You'll see that the paddings are broken in the frontend.
While we could probably fix this somehow by improving the regex, I wonder if it's actually better to not minify ourselves and instead let the minification tools do the work by:
1- Assuming that the file is already minified (and add it to our setup)
2- Introduce a potential convention: load .min.css if the file is available instead of .css.
I personally lean towards 1 and potentially support for core blocks falling back in the blocks registration to unminified files if SCRIPT_DEBUG is true. (I don't like smart behaviors in general and we should better let the right tools handle the right job :) )
There was a problem hiding this comment.
I guess it's fixed by #29554 that said I still think we'd better removing the minification from the framework itself.
There was a problem hiding this comment.
Regarding the bug, #29554 was just merged and fixes it.
I agree however that we should provide minified files, the current ones are OK for debugging but on a real production environment, they are wasteful.
@youknowriad do you know how to do that? I remember I tried to do it in #25220 but ran into issues, mostly because I don't quite understand our tooling and how to use it 😅
There was a problem hiding this comment.
I think it's all in the stylesTransform function we have in the webpack config which in theory should generate minified styles for production code (npm run build) and unmagnified ones for development ( npm run dev)
There was a problem hiding this comment.
Huh... So it already runs as it should, I just always run dev so didn't see that they get minified on production builds 🤦
I'll submit a PR to remove the minify function 👍
There was a problem hiding this comment.
At some point in the future, we might want to ship both files at the same time and introduce the SCRIPT_DEBUG flag support but it's not only about these blocks styles, it's about all JS and styles in Gutenberg plugin.
There was a problem hiding this comment.
At some point in the future, we might want to ship both files at the same time and introduce the
SCRIPT_DEBUGflag support but it's not only about these blocks styles, it's about all JS and styles in Gutenberg plugin.
This is how it works in the WordPress core.
There was a problem hiding this comment.
PR submitted on #29624 👍
There was a problem hiding this comment.
Related issue about shipping .min.* files: #23960
Description
This is a continuation of an effort to improve the sustainability of WordPress FSE themes, which started in #25220
Now that we only load styles for blocks that get rendered, we can continue the process by improving how these files get loaded.
A large majority of blocks have extremely small files for their styles, sometimes no more than a few bytes. Loading an extra file for these doesn't make sense and performance can be improved by inlining them.
This PR sets a filterable maximum size (in bytes) for the total size of CSS that can be inlined.
It then goes through the styles, gets their size and if inlining them doesn't go above the max-inline-size value, then the styles get inlined.
Inlining happens by changing the
srcfrom a URL tofalse, and then adding the contents of the stylesheet usingwp_add_inline_style.If
SCRIPT_DEBUGis not defined (or is false), then in addition to the above, styles that get inlined have their comments and whitespace stripped, with a cleaner and somewhat minified output (for the minification, the regex from https://stackoverflow.com/a/44350195/1965835 was used).Inlining can be disabled completely using the filter:
By default the maximum value of inlined styles is set to 20k, but that's easy to change:
Before deciding which styles should be inlined we build an array with the styles that include the necessary data, order them by size (smaller to larger) and start inlining smallest to larger until we reach the defined limit.
Checklist:
I've included developer documentation if appropriate.I've updated all React Native files affected by any refactorings/renamings in this PR.