-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Break wp-block-library
stylesheet apart into per-block inline styles
#49927
Comments
@oandregal @spacedmonkey @gziolo I'd love to get your thoughts on this idea. |
If I understand correctly, then it should mostly be possible today for Classic Themes to opt-in for features that would enable such setup. It’s how Block Themes work by default after @aristath implemented a set of optimizations summarized in this dev note: We might only need to tweak the default configuration in WordPress core slightly. |
@gziolo Oh wow, I was totally not aware this is already a thing 🤦 Reading that post however it sounds like the block-specific styles are added in the footer? I'm a bit wary about layout shifts since it means the styles are printed notably later than the relevant HTML markup. Is that why it's currently opt-in? cc @aristath |
I believe they're added to the footer only in the Classic theme; because we don't have data on blocks used on the page when the head enqueue styles action is triggered. The block should load per-block styles in the |
We have a few hidden gems that work out of the box for block themes, mainly because it was safer to roll out this way as there were no backward compatibility considerations. We could test whether it would work with all themes maintained in WP core and the themes repository's top 10-20 Classic Themes. If it works seamlessly, then maybe we could turn it on for all themes. |
Exactly.
That would be amazing @gziolo! |
That was actually the 1st implementation I tried when working on splitting the block styles... And it worked brilliantly! I would very much prefer doing that instead of what we do now where we enqueue everything in the To give more context, I basically had 4 implementations to add the styles... 🤣 1. Inline styles the 1st time a block is encountered.This was simply hooking in 2. Use JS to add stylesI loved this method as well... Basically what this was doing, was to add a tiny 3. Add
|
I put together a quick PR WordPress/wordpress-develop#4343 that output the link in the footer if block is found and in the header in the block theme. It seems work well, but putting the styles in the footer means layout shift problems. |
@aristath Thank you for all the context! I wonder why we went with option 4 instead of option 1. I think option 4 is perfectly reasonable for block themes, but for classic themes it is problematic. What was the pushback on option 1? From a layout shift perspective, I can with certainty say that option 1 will work better for classic themes, as putting the styles in the footer is technically too late. For that reason, I think we should go with option 1, at least for classic themes. For block themes, either option 1 or 4 work fine. |
If I remember correctly, the argument was that WP already has a mechanism to enqueue styles using
Option 4 is OK for block themes, but I still believe that option 1 would be preferable...
Agreed. What if we try to do option 1 for classic themes, and then experiment & test performance for block themes as well? If it's not a regression for block themes, then ideally we'd use option 1 for all instances, regardless of the type of theme. |
I could whip up a PR if we want to check inlining the CSS at the time the block gets rendered so we can test things 😉 |
Thanks @aristath, implementing option 1 for classic themes only sounds like a great start! From there we can see how well that works in practice. |
A rough proof of concept that does what was discussed above in included in the snippet below. <?php
if ( ! wp_is_block_theme() ) {
// Load separate block styles.
add_filter( 'should_load_separate_core_block_assets', '__return_true' );
remove_action( 'wp_footer', 'wp_maybe_inline_styles', 1 );
// Inline block styles.
add_filter(
'render_block',
/**
* Filters the content of a single block.
*
* @param string $block_content The block content.
* @param array $block The full block, including name and attributes.
*/
function( $block_content, $block ) {
// Bail early if not a Core block.
if ( ! str_starts_with( $block['blockName'], 'core/' ) ) {
return $block_content;
}
/* This filter is documented in wp-includes/script-loader.php */
$total_inline_limit = apply_filters( 'styles_inline_size_limit', 20000 );
// A static var to keep track of the total inlined CSS size.
static $total_inline_size = 0;
// Get the handle for this block's stylesheet.
$handle = str_replace( 'core/', 'wp-block-', $block['blockName'] );
// Get the WP_Styles object.
$wp_styles = wp_styles();
// Bail early if this block was already processed.
if ( in_array( $handle, $wp_styles->done, true ) ) {
return $block_content;
}
// Bail early if the block doesn't have a stylesheet.
if ( empty( $wp_styles->registered[ $handle ] ) ) {
return $block_content;
}
$path = wp_styles()->get_data( $handle, 'path' );
$after = wp_styles()->get_data( $handle, 'after' );
if ( is_array( $after ) ) {
$after = implode( '', $after );
}
// Bail early if the stylesheet doesn't have a "path" defined,
// or if there are no inline styles attached to it.
if ( ! $path && ! $after ) {
return $block_content;
}
// Get the CSS for this block's stylesheet.
$css = file_exists( $path ) ? file_get_contents( $path ) : '';
// Add the "after" styles.
$css .= $after;
// Get the size of the CSS.
$styles_size = strlen( $css );
// Bail early if the CSS is empty.
if ( 0 === $styles_size ) {
$wp_styles->done[] = $handle;
return $block_content;
}
// Check if adding these styles inline will exceed the limit.
// If it does, then bail early.
if ( $total_inline_size + $styles_size > $total_inline_limit ) {
return $block_content;
}
$wp_styles->done[] = $handle;
$block_content .= '<style id="' . $handle . '-inline-css">' . $css . '</style>';
return $block_content;
},
PHP_INT_MAX,
2
);
} How to test the above code
NotesWe already have a add_filter( 'styles_inline_size_limit', function() {
return MB_IN_BYTES;
} ); |
@aristath Thanks for sharing this snippet, that looks like a great start to me! Two points of feedback:
@spacedmonkey Looking at your branch, that's another interesting variant, but it looks like it still prints the entire @aristath I'm still not sure I fully understand the relationship between the |
Yes, that's what Core does too. We have the
You're absolutely right, I missed that. Fixed by imploding the values 👍
The
When we add |
@aristath Thank you for all the clarification, that makes sense! |
As per yesterday's performance meeting, I submitted a PR on #50087 |
Great work on #50087. It looks great! So the currently implements, add inline styles to the contents of say post content. Would this be an issue if you are using WP headless, say in a react site or a mobile app. Those style tags might break styling. |
@aristath I left some feedback and performance benchmarking data on #50087, it looks great so far from a client-side performance perspective. I also really like where it's going overall. That said, it would be great if we can find ways to make the additional server-side logic more efficient. Separately, I was just thinking maybe we could also address https://core.trac.wordpress.org/ticket/56990 with this? If we know what blocks are rendered, we could include those few styles inline as well instead of always loading this small |
I believe I was one of those who was expressing resistance here, but not because I don't love the idea! Option 1 (and Option 3) both involve marking a stylesheet as being For example, in my syntax-highlighting-code-block plugin, it currently is waiting to print the stylesheets for syntax highlighting until a Code block appears on the page (using the same approaches as Options 1 & 3). The The reason is that some plugins, like the Fixed TOC table of contents plugin, will call The other concern I have about inlining is it could cause the CSS selectors to also break. Let's say you have two blocks, a paragraph block and a list block: <p>...</p>
<ol>
<li>...</li>
</ol> And the stylesheets for both get inlined right before each block: <style> p {} </style>
<p>...</p>
<style> ol {} </style>
<ol>
<li>...</li>
</ol> Let's also say you have a stylesheet in your theme that decides you want to apply some styling to lists that appear right after paragraphs: p + ol,
p + ul {
/* ... */
} The inlining of the CSS with the blocks unfortunately breaks these CSS selectors since the To inline CSS by injecting |
The only alternative I can think of which would work for classic themes would be to incorporate output buffering of the template output, which would effectively open the door to doing the same optimizations possible with the block themes (i.e. the block styles printed in the footer could be moved to the Nevertheless, with the |
Actually, no string manipulation would be needed at all with output buffering. If output buffering starts at the |
So at the moment, we have two places where style / scripts can be output on the page, header and footer. If you enqueue a style after header are output, it fallback to outputting in the footer. What if we add a new Another idea, would be to loop through posts in the loop and then see if they blocks and output styles for only blocks used in the loops post content. For blocks in widgets we could add styles before the sidebar is output. For other blocks on the page like other WP_Query, this would fallback to outputting in the footer. None of these solutions feel great. Any solution would give theme developers control of the markup that is generated. That control, might mean we need to make this opt-in. |
Yeah, this is essentially the same situation as we have today, where the styles are printed in the footer. It would just mean the styles are printed a bit higher up, but there is still the possibility for FOUC.
One issue with this is that even though a post may be in the loop, this doesn't mean that each post's content will actually be rendered on the page. Consider the case of archive templates that use IMO, the only/best way to solve this problem is to utilize output buffering. I'm going to do some more research in that area. |
So if If we have I would love to see all themes have this functionality, is this something we could look into changing for all themes? Thoughts @felixarntz |
@spacedmonkey The reason it is disabled by default for non-block themes is because the split up styles end up getting printed in the footer, thus causing the possibility of FOUC. This isn't an issue for block themes since the entire page is rendered in memory before being sent to the client, and thus the split-up styles can still be inserted into the |
I put together a little a POC plugin. https://gist.github.com/spacedmonkey/29c2af943da9d729138eb47ff7a18cff. This does two things
To be clear, that is not all blocks that could be on a page, blocks could come from other places, like custom WP_Query. But it will get all the likely blocks. Any other blocks styles will render in the footer. Not perfect, but could be workable. |
As a user, I understand that I cannot directly participate in your conversation, but I'm glad to see someone else who shares my concern about performance issues. In an ideal scenario, Gutenberg could be designed to allow users to create their own style packs? Choose the desired blocks (already available). Our idea is that no matter how it is designed, full compatibility cannot be achieved because there are too many scenarios. Sometimes, we want to use the classic editor and the block editor together. Would it be too challenging to develop an automated style pack generation feature? |
What problem does this address?
Block themes do a great job conditionally enqueueing only the CSS that is actually needed using inline
<style>
tags. This is facilitated by the benefit of block themes generating their entire markup prior to outputting it, which allows us to detect what is used in the page and then conditionally including the necessary CSS in the<head>
, i.e. still before the relevant content is rendered in the page.While block themes are the future of WordPress, classic themes are still heavily used, and at the moment they always load the entire
wp-block-library
stylesheet, regardless of how much of the CSS rules in there are being used. This is bad for performance, and while for classic themes it may not seem as intuitive as for block themes to only load the CSS that's needed per block, there may still be a good way to do so.At a minimum, this will involve breaking apart the single
wp-block-library
stylesheet that is built in theblock-library
package into distinct stylesheets per block type. This is already the case in the source code here, but we'd have to probably update the build process to no longer create just a single CSS file with everything.What is your proposed solution?
Classic themes are procedurally output, i.e. we can't enqueue something in the
<head>
based on the page content, since by the time we know that, we're already past rendering the<head>
. However, we can still conditionally output styles per block as we encounter the blocks on the page:<style>
tags are perfectly reasonable to be included anywhere in the page. They do not have to appear in the<head>
, and in fact loading them later can even be good for performance, as applied in the best practice of inlining critical CSS and loading the remaining CSS later.wp-block-library
stylesheet that has basic styles for tons of blocks, all of these blocks could have their individual block stylesheet inblock.json
.core/gallery
block is encountered for the first time in a page, we include a<style>
tag with the contents of that block's stylesheet (based onpackages/block-library/src/gallery/style.scss
) right before the block's actual output.With the above, we will be able to load substantially less CSS, particularly as many pages wastefully load the entire
wp-block-library
file today even though they use only a fraction of the blocks referenced in the CSS rules there.Important considerations
Note that using inline stylesheets and including them right where the block is rendered is crucial: If CSS for visible content gets loaded too late, there is a risk of layout shifts which make for a bad user experience, so we can't just put the CSS in the footer, as that would mean the CSS is loaded too late. It's crucial we include the CSS right before the block is rendered. Additionally, we also shouldn't use external stylesheets as that would mean potentially a lot of extra stylesheets for little CSS, at which point it makes more sense to inline the CSS rules, similarly to how it is already done for block themes.
The text was updated successfully, but these errors were encountered: