-
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
Tree-shaking block styles on the frontend #41020
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +54 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
0f70903
to
f6f4fc9
Compare
Would this have any eventual overlap with/effect on the exploratory work in
I believe the premise is to move some block CSS rules to block.json and then merge with theme.json styles. cc @oandregal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fascinating exploration @aristath, nice work digging into the performance / fine-tuning the styles that get output to the site frontend!
What sort of impact will this change have for plugins that load block content in asynchronous contexts (for example, endless scroll plugins, or any other use case where post content is fetched via the REST API and then appended to the page)?
We currently have an open issue (#35376) for Layout styles (or anything that's currently output to separate style
tags when individual blocks are rendered) where certain styles are unavailable in async contexts, so that the Post Content block does not render correctly within the editor, but this'll also apply to those endless scroll / async loading plugins, too. I previously tried a few different ideas to solve this problem, but unfortunately haven't landed on a good approach yet, but I'd be very keen to hear if you have any thoughts on how we might solve for this if we tree-shake block style output.
To try to distill the use case / question:
"As a plugin developer, on an already loaded page of the site frontend, if I asynchronously load some extra block content to be appended to the page, how do I ensure I have the styles in place for that extra markup?"
There would be no overlap between these two.
This doesn't work now either... If in a block theme, there is an infinite scroll plugin (or any other method that loads content async), and that plugin loads a block that doesn't already exist on the page, then styles for that block don't get loaded. |
Great point, thanks for clarifying.
Absolutely. I mostly wanted to ask the question to check whether or not the proposed change will make it harder or easier for us when it does come time to solve it, and it sounds like it won't make too much of a difference. If we think the difference is going to be negligible then let's not hold up introducing the performance enhancement!
It's a tricky issue, because there's lots of infinite scroll plugins in the plugin directory with large numbers of active installations (e.g. 50k+), but that are intended for classic themes. If it's difficult or impossible to implement in blocks-based themes, then we're sort of in a position where we know lots of sites use infinite scroll, but there isn't yet an easy way for folks to build it — so we have a potential gap where I think the current state of FSE and global styles means that it isn't easy for plugin developers to enthusiastically jump into this space. But that's very much beyond the scope of this PR 😀 and probably more relates to discussions in #38224 on block hydration.
The main concrete scenario we have at the moment is the problem of being able to accurately preview post content in the Post Content block in the site editor. Ideally we'd be rendering a 1:1 representation of the post, but as in #35376, there are some styles already that don't appear correctly there. So again, not a blocker for this PR, just an issue that I'd be keen for us to come up with a long-term solution to, or to keep in the back of our minds if we make changes that might make it trickier to fix.
Oh, the injection ideas in your earlier PRs are really interesting, too. Thanks again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cools stuff 👏🏻 I think the code looks good in terms of functionality but I am unsure about:
- the "styledClasses" name in block json schema
- the editor / front end disconnect
Am I missing something or if I want styledClasses then every style of my block has to be one of these classes?
aedddfe
to
8fe147d
Compare
Thank you @draganescu !
In the editor, we have no way to "lazy-load" or inject styles JIT as a block gets added. That's why the current behavior of the editor is to just load a single file for all blocks (
Well... not quite. In the I agree that the |
@aristath do you think we should try to have all the css files inside a separate directory? Just for easier navigation and better file organization, as the main block directory could end up with many css files. |
I think that would be nice, yes. It's what I initially tried to do in my first commit here. However, I could not get the build processes right at the time, and files in a subfolder were not getting compiled. I'll take another look and try to get them to get properly compiled in a subfolder, but it would be great if someone with more experience in webpack & our tooling can lend a hand 👍 |
Update: Moved the style partials to a |
@aristath, thank you so much for taking a stab at another performance optimization for blocks. It's really interesting to see how you try to find a way to ship only those styles provided for the block that is really used on the front end. While the implementation proposed is technically working and I believe could be shipped as-is, I would like us to discuss what other alternatives we have here. Can we avoid introducing another Block API related to styles? We have already several ways to control styles:
Is there a way to update one of the existing methods to achieve similar goals? Using class names as the way to perform all the optimizations is also something I want to understand better. There was a lengthy discussion in #38998 about standardizing block markup, CSS classes, and so on. The consensus was that there is no plan to promote the existing class names used with core blocks for some styling purposes to public APIs as explained by @mtias in #38998 (comment). Aren't you afraid that the API proposed would increase the importance of using standardized class names in block markup? |
Thank you for the feedback @gziolo! Those are some interesting questions...
I wouldn't call this one an "API"... When backported to Core it could be a part of one of the existing functions without exposing anything additional. Using the Regarding class names: Technically the implementation does not exactly use class names... As mentioned in my comment above, it can be any string that is included in the block markup. So if for example next year we move away from a class name
well... The partial styles will be on a per-block basis, so each block is treated uniquely depending on its scenario. I don't think that it will make standardization any more or less important, especially if we find a more suitable name for the |
Currently, it's a top-level field in How would you implement it in WordPress core differently?
That's interesting. So
Elements are already a thing in |
Thank you @gziolo for the feedback! Taking it into account, I pushed a commit that moved the implementation inside I think this change also resolves the issue about using the term "class" in there... 😅 |
One thing to look at is that a lot of the block styles we bundle in core predated style attributes. Same for core style variations that are opaque (i.e. their styles are not expressed as style attributes, which means they are not composable). Instead of thinking about having multiple style variations we should look at decomposing them into attributes, so we can migrate all of those into style properties and out of the opaque classes in most cases. |
f648c92
to
f88de84
Compare
I think this solution is acceptable for core blocks, where the developer experience is not that important, and we need to squeeze any performance gain. But I don't like the idea of adding more APIs to our already long list. In my opinion, we should aim to remove APIs, not to add more. Tree-shaking is an automatic process handled by compilers/bundlers, not by the developers. We should invest more time investigating ways to absorb these optimizations in WordPress in a way that is transparent to the developer, works by default and doesn't need a public API. |
Perhaps "tree-shaking" was a poor choice of words 😅 |
Even if it's not a new property or function, it's yet another API in the sense that developers need to know about that option and how they can use it. But hey, I don't pretend to go against this PR if it can squeeze a few kilobytes out of core blocks CSS for each page load. It's just that I think that we need to find ways to reduce the number of APIs/concepts developers are exposed to, especially when they are related to performance optimizations 🙂 |
No argument there... Any suggestions? |
If Gutenberg were to absorb these optimizations, it must do so programmatically. Then the question becomes: what information does Gutenberg need to do this optimization automatically? As an example, this issue is also related to CSS tree-shaking but is more aligned with the mindset I'm proposing. We give Gutenberg all the information (the block CSS, the theme.json and the Global Styles), and it decides what to render in each case. Of course, adding CSS in a JSON is a step backward in terms of developer experience, so the next step should be to figure out how developers could write their CSS in CSS files. Fortunately, turning CSS files into objects is something that has been widely explored in JS framework compilers and CSS-in-JS solutions. It can even be done with a regexp (code, tests). So at least we know it's doable. But as I said, Ari, I don't want to block this issue. I think it still adds value to Gutenberg. It was just a comment about a mindset I hope we could embrace when thinking about optimizations, to slowly improve Gutenberg's developer experience over time 😊 |
Just adding my 2 cents that since the whole @aristath would you say #34180 opens the door for better approaches to tree shaking block styles on the front end? If, and only if, yes, then could that path be considered and improved towards optimisation? |
Co-authored-by: Alex Lende <[email protected]>
f88de84
to
3531b1e
Compare
What?
In #25220 we split the CSS files for blocks, so block-themes can load separate files per-block, depending on what blocks are present on a page.
This PR takes the concept further, and allows loading CSS files depending on the actual block contents/classes etc.
Why?
Block stylesheets contain all styles for all scenarios a block may encounter. Some of those scenarios are not that frequent, yet their styles must always be loaded because we currently have no way to differentiate what's used and what's not.
This results is loading way more CSS than what is actually needed in a page, wasting resources.
The implementation in this PR allows us to do some tree-shaking and only load the styles that are actually used on a page.
The benefits will be improved performance for site visitors - especially for block themes, and improved sustainability for the WP ecosystem.
How?
The PR adds a
wp_maybe_inline_block_style_parts
function. That function checks for astyle.[].parts
(array) entry inblock.json
files (see example in the included paragraph block tweaks)It then adds a filter on
render_block
, and if the block contains the parts from the above array, it includes the corresponding CSS file (namedstyles/{$part}.css
).This PR also includes changes to the
core/paragraph
block, splitting it to separate files, to make the implementation clearer and easier to test.Testing Instructions
Add a paragraph block on a page, load the post on the frontend and ensure that if you don't use a dropcap, the CSS for it doesn't get loaded. If you edit the post and add a dropcap, the CSS for it should then become available on the frontend.
This change should not have any impact on the editor, this is a frontend-only improvement.