Block Visibility: Avoid resetting asset queues prior to block rendering and add wp_enqueue_scripts special case
#10252
Conversation
westonruter
changed the title
westonruter
changed the title
wp_head special case
|
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. |
|
I've tested this on WordPress.org, and done a diff of the output before changes, verses with trunk with this PR, and the only diff is a The approach of dequeuing instead of conditionally enqueueing assets does seem a bit weird at first, but It does seem like the best approach for back-compat. The only alternative that I can think of would require a far more indepth change to how WP_Dependency operates, effectively adding a Having to check for |
src/wp-includes/class-wp-block.php
Outdated
| global $post; | ||
|
|
||
| // Capture the current assets queues and then clear out to capture the diff of what was introduced by rendering. | ||
| $before_wp_head_count = did_action( 'wp_head' ); |
There was a problem hiding this comment.
In @dd32's code, it appears that the specific problem is not wp_head but rather wp_enqueue|register_style|scripts. It just happens that wp_head fires them.
It could be best to test for the specific rather than the general. 🤷♂️
There was a problem hiding this comment.
The issue (as I found) is with wp_print_scripts() and wp_print_styles(), that either of these (especially the former) may not print anything during wp_head, since what is enqueued may get printed in wp_footer. We could change this to look if the wp_print_scripts or wp_print_styles actions fired, but they don't always fire.
Or actually, maybe it would be better to check if the wp_enqueue_scripts action fired. As I think about it, the issue is that plugins and themes are being invited to enqueued scripts and styles, but if nothing is printed then we undo the enqueueing. So this needs to be prevented.
There was a problem hiding this comment.
I've made that change in 7f8f721. That feels better and makes much more sense.
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
…into trac-63676-fixing-blocks-containing-wp-head
westonruter
changed the title
wp_head special casewp_enqueue_scripts special case
…to enqueue assets for non-hidden blocks. This eliminates constant emptying out of the queues for styles, scripts, and script modules before rendering each block. This ensures that `wp_script_is()`/`wp_style_is()` will return true for assets that are actually enqueued. The `WP_Script_Modules::$queue` member which was made public in [60930] is now made private in favor of a `WP_Script_Modules::get_queue()` method, since there is no need to clear out the queue before rendering each block and restore after the rendering is complete. Finally, as a very special case for unusual blocks which contain `wp_head()`, a check is done to see if the `wp_enqueue_scripts` action occurred during the rendering of a block; if so, then no assets will be dequeued even if no markup is rendered in the block, since it may be that a script was enqueued for the footer and not the head. Developed in #10252 Follow-up to [60930]. Props westonruter, dd32, peterwilsoncc, nikunj8866, krupajnanda. Fixes #63676. git-svn-id: https://develop.svn.wordpress.org/trunk@60951 602fd350-edb4-49c9-b593-d223f7449a82
…to enqueue assets for non-hidden blocks. This eliminates constant emptying out of the queues for styles, scripts, and script modules before rendering each block. This ensures that `wp_script_is()`/`wp_style_is()` will return true for assets that are actually enqueued. The `WP_Script_Modules::$queue` member which was made public in [60930] is now made private in favor of a `WP_Script_Modules::get_queue()` method, since there is no need to clear out the queue before rendering each block and restore after the rendering is complete. Finally, as a very special case for unusual blocks which contain `wp_head()`, a check is done to see if the `wp_enqueue_scripts` action occurred during the rendering of a block; if so, then no assets will be dequeued even if no markup is rendered in the block, since it may be that a script was enqueued for the footer and not the head. Developed in WordPress/wordpress-develop#10252 Follow-up to [60930]. Props westonruter, dd32, peterwilsoncc, nikunj8866, krupajnanda. Fixes #63676. Built from https://develop.svn.wordpress.org/trunk@60951 git-svn-id: http://core.svn.wordpress.org/trunk@60287 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…to enqueue assets for non-hidden blocks. This eliminates constant emptying out of the queues for styles, scripts, and script modules before rendering each block. This ensures that `wp_script_is()`/`wp_style_is()` will return true for assets that are actually enqueued. The `WP_Script_Modules::$queue` member which was made public in [60930] is now made private in favor of a `WP_Script_Modules::get_queue()` method, since there is no need to clear out the queue before rendering each block and restore after the rendering is complete. Finally, as a very special case for unusual blocks which contain `wp_head()`, a check is done to see if the `wp_enqueue_scripts` action occurred during the rendering of a block; if so, then no assets will be dequeued even if no markup is rendered in the block, since it may be that a script was enqueued for the footer and not the head. Developed in WordPress/wordpress-develop#10252 Follow-up to [60930]. Props westonruter, dd32, peterwilsoncc, nikunj8866, krupajnanda. Fixes #63676. Built from https://develop.svn.wordpress.org/trunk@60951 git-svn-id: https://core.svn.wordpress.org/trunk@60287 1a063a9b-81f0-0310-95a4-ce76da25c4cd
3 participants
This is a follow-up to #9213 (r60930 & 9d03e8e) which broke asset enqueues on WPOrg, as reported by @dd32:
This PR takes an alternative approach to not clear out the queue before each block is rendered. Since blocks are rendered in depth-first order, simply capturing the enqueued assets before rendering and then comparing with the assets enqueued after a block (or nested block tree) is rendered should be sufficient. When no content is rendered in the block, then the logic has been updated to dequeue what had newly been enqueued (rather than to merge queues of the previous queue and the new queue).
As an added bonus, this ensures that
wp_script_is( 'admin-bar', 'enqueued' )works as expected in the block render callbacks/filters.Then there is a very special case added for the sake of themes which may include a block which specifically renders
wp_head(): in the case of enqueueing a script there which is only printed in the footer, the result ofwp_head()may be no output at all (since the script will be printed atwp_footer()instead. To account for this, now the logic for checking whether to dequeue assets is skipped if thewp_headaction happened during the block's rendering.This PR also undoes the exposing of
WP_Script_Modules::$queuemember variable in favor of aWP_Script_Modules::get_queue()method, since the queue no longer needs to be easily written to directly; nowwp_dequeue_script_module()is used to remove enqueued script modules when necessary.Trac ticket: https://core.trac.wordpress.org/ticket/63676
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.