Avoid enqueueing scripts and styles for a block unless content is rendered #9213
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
The code looks good and it makes sense.
Tested and works as expected.
This should reduce the CSS significantly for sites using block themes where the templates include blocks that don't actually print anything on the page because of the structure of individual posts/pages etc 👍
|
This should work great for CSS because you need at least one tag so the rules make sense. I'm curious how it will play out with JavaScript. For the Interactivity API, you need to have a wrapping element that enables an interactive region so the proposed logic fits nicely. For regular scripts, there might be a more nuanced reality. In majority of cases it should work correctly, but I'm not entirely sure what the long tail could be. Is it possible that a custom block renders nothing but still runs some arbitrary JS code? |
I think this corresponds to the feedback left by @dd32:
And likewise from @aaronjorbin:
I suppose then the value of All: How about something like this? $processor = new WP_HTML_Tag_Processor( $content );
$enqueue = $processor->next_tag();
/**
* Filters whether to enqueue assets for a block which has no rendered content.
*
* @since n.e.x.t
*
* @param bool $enqueue Whether to enqueue assets.
* @param string $block_name Block name.
*/
$enqueue = (bool) apply_filters( 'wp_enqueue_empty_block_content_assets', $enqueue, $block_name );
if ( $enqueue ) {
/* ... */
} |
|
@gziolo @dd32 @aaronjorbin How about f3209f6? With that in place, you can force the assets for a rendered block to be enqueued via the add_filter(
'enqueue_empty_block_content_assets',
function ( $enqueue, $block_name ) {
if ( 'core/post-featured-image' === $block_name ) {
$enqueue = true;
}
return $enqueue;
},
10,
2
);If that looks good, I'll proceed with adding tests so that this can be advanced for review. |
|
The solution proposed, which includes a filter that allows for enqueuing assets even when no content is printed, seems to be spot on. There may be some backward compatibility considerations, but these should be noted in the dev note with details on how to address them, if necessary. |
|
Thank you! I'll proceed with adding tests so this can be moved to review and commit. |
|
In 59c3543 I've started adding tests, but I found a complication related to inner blocks. If an inner block has its assets enqueued, but the outer block is filtered to be hidden, then ideally the inner block's assets should be omitted from being rendered. This is not the case right now, however. |
|
The challenge is that rendering occurs from the bottom up. It begins with the deeply nested inner blocks before rendering their ancestors. That's why, at the time of rendering an inner block, you don't know whether any of the parent blocks might decide to, for some reason, render nothing. |
There was a problem hiding this comment.
The changes here are discussed in Slack: https://wordpress.slack.com/archives/C02QB2JS7/p1759958061791369
They are needed to allow npm run test:php -- --group=blocks to run without errors. Otherwise, this occurs:
Tests_Blocks_Editor::test_get_block_editor_settings_overrides_default_settings_all_editors
Attempt to read property "registered" on null
/var/www/src/wp-includes/block-editor.php:317
/var/www/src/wp-includes/block-editor.php:633
/var/www/tests/phpunit/tests/blocks/editor.php:406
It seems _wp_get_iframed_editor_assets() is assuming the $wp_scripts and $wp_styles globals are defined when they may not be, and normally when the entire test suite runs one of the other tests will have populated these globals without cleaning up after itself. But when this group of tests runs in isolation, the globals aren't defined, and thus the error occurs.
I think the function should be made more robust:
diff --git a/src/wp-includes/block-editor.php b/src/wp-includes/block-editor.php
index 6f5720ec21..19edc778f7 100644
--- a/src/wp-includes/block-editor.php
+++ b/src/wp-includes/block-editor.php
@@ -302,8 +302,8 @@ function _wp_get_iframed_editor_assets() {
global $wp_styles, $wp_scripts;
// Keep track of the styles and scripts instance to restore later.
- $current_wp_styles = $wp_styles;
- $current_wp_scripts = $wp_scripts;
+ $current_wp_styles = wp_styles();
+ $current_wp_scripts = wp_scripts();
// Create new instances to collect the assets.
$wp_styles = new WP_Styles();
…account for empty content Co-authored-by: Peter Wilson <[email protected]>
…content is not empty
I reverted my naïeve implementation and I've taken another stab at it to account for the depth-first traversal of nested blocks: 90a9b4e. What it does now is before a block is rendered, it captures the queues for the enqueued styles, scripts, and script modules and then empties them out. Then it goes forward with rendering the inner blocks and the block's own content. Then it captures the queues again to find out which new assets were enqueued and restores the original queues. Then it checks if the rendered block content is not empty, and if so (or else the filter allows), it will proceed to merge those newly enqueued assets with the assets previously-enqueued when the block was being initially rendered. Re-testing my original scenario: Given the Hello World
To make it easier to compare the difference in the amount of CSS being added to the page, I've eliminated the inline CSS limit: add_filter(
'styles_inline_size_limit',
static function (): int {
return 0;
}
);
|
CSS Coverage ReportCoverage report taken after page reload on a desktop viewport without doing any interactions. Before: 39% of 136 kB used
After: 53% of 100 kB used
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@dmsnell FYI: Given your mission for block parsing efficiency, this seems like something you'd want to review. |
…into trac-63676
|
Hello @westonruter My Testing Approach:
My Finding:
Now, visually, I don't see any major differences here, but I observed that @westonruter Please confirm if the approach is correct. |
|
@kjnanda yes, that is correct! You can also try nesting some block deep inside of another block, like a Video inside of a column inside of a Group. If you hide the Group block, then the CSS for the Columns block and Video block should both be omitted from the page. In all cases, the visual appearance of the page should remain the same (except for whether the block is hidden or not of course). You could also try adding an Image block with lightbox enabled (expand on click). If that is the only Image block on the page, if you hide the the block then you should that it's view script module is omitted from the page. |
|
@westonruter Noted. And this is working as expected. |
Great idea! It should cover all types of enqueuing I can think of for styles, scripts, and script modules when rendering blocks. It maps enqueued assets to the specific block well, enabling detailed control over what gets added to the global queue once the block tree is marked as non-empty and needs these assets. That fully addresses all the concerns raised previously 🚢 |
| * @covers WP_Script_Modules::queue | ||
| * @covers WP_Script_Modules::dequeue | ||
| */ | ||
| public function test_direct_queue_manipulation() { |
There was a problem hiding this comment.
As this contain multiple assertions, each of them will need a custom message parameter.
|
|
||
| /** | ||
| * Holds the script module identifiers that were enqueued before registered. | ||
| * An array of IDs for queued script modules. |
There was a problem hiding this comment.
Why is there a need to switch from keying by script ID to an array of script ID values?
It's adding a bunch of work to avoid duplicates that I think could be more easily covered by using a method to add the new asset in the wp_block class.
As developer interfaces go, I also think that an ::add_to_queue method would be more helpful that requiring third party devs do an array_unique( array_merge() ) each time they modify the queue.
Even if we advice that manipulating the queue in this manner is discouraged, it will be done.
There was a problem hiding this comment.
This brings WP_Script_Modules in alignment with WP_Scripts and WP_Styles which both expose a $queue member variable. It allows us to easily obtain the list of IDs enqueued, and then merge them.
For WP_Scripts and WP_Styles it is primarily for reading, but it is also used for writing here (now in 6.9):
wordpress-develop/src/wp-includes/media.php
Lines 2113 to 2114 in b3c3c8e
In reality, the array_unique() is not needed because WP_Scripts, WP_Styles, and WP_Script_Modules all obtain the unique items when processing anyway (or else keep track via done).
We could introduce a magic setter function for the queue member (which we could then make private). This would have the benefit of not only ensuring uniqueness, but we could also throw an error if anything other than an array of strings is attempted to be set.
There was a problem hiding this comment.
Thanks, let's go with consistency.
Co-authored-by: Peter Wilson <[email protected]
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
…s nothing to merge
This change prevents scripts, styles, and script modules from being enqueued for blocks that do not render any HTML content. This is common for hidden blocks or blocks like the Featured Image block when no image is present. This change reduces the amount of unused CSS and JavaScript on a page, improving performance. A new filter, `enqueue_empty_block_content_assets`, is introduced to allow developers to override this behavior and enqueue assets for empty blocks if needed. The implementation involves capturing the asset queues before and after a block is rendered. The newly enqueued assets are only merged if the block's rendered content is not empty. This is done recursively for nested blocks to ensure that assets for inner blocks are also not enqueued if a parent block is hidden. Developed in #9213. Props westonruter, aristath, peterwilsoncc, gziolo, krupajnanda, dd32, jorbin. See #50328. Fixes #63676. git-svn-id: https://develop.svn.wordpress.org/trunk@60930 602fd350-edb4-49c9-b593-d223f7449a82
This change prevents scripts, styles, and script modules from being enqueued for blocks that do not render any HTML content. This is common for hidden blocks or blocks like the Featured Image block when no image is present. This change reduces the amount of unused CSS and JavaScript on a page, improving performance. A new filter, `enqueue_empty_block_content_assets`, is introduced to allow developers to override this behavior and enqueue assets for empty blocks if needed. The implementation involves capturing the asset queues before and after a block is rendered. The newly enqueued assets are only merged if the block's rendered content is not empty. This is done recursively for nested blocks to ensure that assets for inner blocks are also not enqueued if a parent block is hidden. Developed in WordPress/wordpress-develop#9213. Props westonruter, aristath, peterwilsoncc, gziolo, krupajnanda, dd32, jorbin. See #50328. Fixes #63676. Built from https://develop.svn.wordpress.org/trunk@60930 git-svn-id: http://core.svn.wordpress.org/trunk@60266 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This change prevents scripts, styles, and script modules from being enqueued for blocks that do not render any HTML content. This is common for hidden blocks or blocks like the Featured Image block when no image is present. This change reduces the amount of unused CSS and JavaScript on a page, improving performance. A new filter, `enqueue_empty_block_content_assets`, is introduced to allow developers to override this behavior and enqueue assets for empty blocks if needed. The implementation involves capturing the asset queues before and after a block is rendered. The newly enqueued assets are only merged if the block's rendered content is not empty. This is done recursively for nested blocks to ensure that assets for inner blocks are also not enqueued if a parent block is hidden. Developed in WordPress/wordpress-develop#9213. Props westonruter, aristath, peterwilsoncc, gziolo, krupajnanda, dd32, jorbin. See #50328. Fixes #63676. Built from https://develop.svn.wordpress.org/trunk@60930 git-svn-id: https://core.svn.wordpress.org/trunk@60266 1a063a9b-81f0-0310-95a4-ce76da25c4cd
| $this->registered[ $id ]['enqueue'] = false; | ||
| } | ||
| unset( $this->enqueued_before_registered[ $id ] ); | ||
| $this->queue = array_diff( $this->queue, array( $id ) ); |
There was a problem hiding this comment.
Quick Q: After the change it will bypass the updating on $this->registered[ $id ]['enqueue'] to false. Does it accepted behaviour?
There was a problem hiding this comment.
The enqueue property of the script module array has been removed entirely. Now the enqueued state is exclusively captured in the enqueue class member variable, same as with WP_Scripts. Since all of this was private before, there shouldn't be any back-compat issues.
|
This caused a breakage on dotorg: https://core.trac.wordpress.org/ticket/63676#comment:28 Follow-up PR to fix: #10252 |
thanks for the ping, @westonruter. if I’m understanding it properly, this quieting activity is occurring after the blocks have already been parsed, in which case I don’t think it speaks to that area of efficiency gains. now, on the other hand, if I missed something because it’s not in the diff context and we are making a separate pass with // Get the list of unique block types present within a post.
$processor = new WP_Block_Processor( $post_content );
$block_types = array();
while ( $processor->next_block() ) {
$block_types[ $processor->get_block_type() ] = true;
}
return array_keys( $block_types ); |
7 participants
Trac ticket: https://core.trac.wordpress.org/ticket/63676
See new description in #9213 (comment)
Old Description
Given a published
poston thesingletemplate on the Twenty Twenty-Five theme where:This PR omits the CSS (and scripts) for the unrendered blocks, as seen from this diff:
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.