Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.6k
Avoid enqueueing scripts and styles for a block unless content is rendered#9213
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
Changes from all commits
40f9bce0e5a3f2f3209f659c3543ccc9126648713bde22ae9556c0740ba3ccb0027b467358d5b90a9b4e0bbcaf6ce4789b7460f61df2d0182ac38f5c9b892e29a46d473a899ae7daf5192c44f4File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -23,12 +23,12 @@ class WP_Script_Modules { | ||
| private $registered = array(); | ||
| /** | ||
| * Holds the script module identifiers that were enqueued before registered. | ||
| * An array of IDs for queued script modules. | ||
| * | ||
| * @since 6.5.0 | ||
| * @var array<string, true> | ||
| * @since 6.9.0 | ||
| * @var string[] | ||
| */ | ||
| private $enqueued_before_registered = array(); | ||
| public $queue = array(); | ||
| /** | ||
| * Tracks whether the @wordpress/a11y script module is available. | ||
| @@ -122,7 +122,6 @@ public function register( string $id, string $src, array $deps = array(), $versi | ||
| $this->registered[ $id ] = array( | ||
| 'src' => $src, | ||
| 'version' => $version, | ||
| 'enqueue' => isset( $this->enqueued_before_registered[ $id ] ), | ||
| 'dependencies' => $dependencies, | ||
| 'fetchpriority' => $fetchpriority, | ||
| ); | ||
| @@ -213,13 +212,11 @@ public function set_fetchpriority( string $id, string $priority ): bool { | ||
| * } | ||
| */ | ||
| public function enqueue( string $id, string $src = '', array $deps = array(), $version = false, array $args = array() ) { | ||
| if ( isset( $this->registered[ $id ] ) ) { | ||
| $this->registered[ $id ]['enqueue'] = true; | ||
| } elseif ( $src ) { | ||
| if ( ! in_array( $id, $this->queue, true ) ) { | ||
| $this->queue[] = $id; | ||
| } | ||
| if ( ! isset( $this->registered[ $id ] ) && $src ) { | ||
| $this->register( $id, $src, $deps, $version, $args ); | ||
| $this->registered[ $id ]['enqueue'] = true; | ||
| } else { | ||
| $this->enqueued_before_registered[ $id ] = true; | ||
| } | ||
| } | ||
| @@ -231,10 +228,7 @@ public function enqueue( string $id, string $src = '', array $deps = array(), $v | ||
| * @param string $id The identifier of the script module. | ||
| */ | ||
| public function dequeue( string $id ) { | ||
| if ( isset( $this->registered[ $id ] ) ) { | ||
| $this->registered[ $id ]['enqueue'] = false; | ||
| } | ||
| unset( $this->enqueued_before_registered[ $id ] ); | ||
| $this->queue = array_diff( $this->queue, array( $id ) ); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quick Q: After the change it will bypass the updating on MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The | ||
| } | ||
| /** | ||
| @@ -245,8 +239,8 @@ public function dequeue( string $id ) { | ||
| * @param string $id The identifier of the script module. | ||
| */ | ||
| public function deregister( string $id ) { | ||
| $this->dequeue( $id ); | ||
| unset( $this->registered[ $id ] ); | ||
| unset( $this->enqueued_before_registered[ $id ] ); | ||
| } | ||
| /** | ||
| @@ -304,9 +298,9 @@ public function print_enqueued_script_modules() { | ||
| * @since 6.5.0 | ||
| */ | ||
| public function print_script_module_preloads() { | ||
| foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ), array( 'static' ) ) as $id => $script_module ) { | ||
| foreach ( $this->get_dependencies( array_unique( $this->queue ), array( 'static' ) ) as $id => $script_module ) { | ||
| // Don't preload if it's marked for enqueue. | ||
| if ( true !== $script_module['enqueue'] ) { | ||
| if ( ! in_array( $id, $this->queue, true ) ) { | ||
| echo sprintf( | ||
| '<link rel="modulepreload" href="%s" id="%s"%s>', | ||
| esc_url( $this->get_src( $id ) ), | ||
| @@ -345,7 +339,7 @@ public function print_import_map() { | ||
| */ | ||
| private function get_import_map(): array { | ||
| $imports = array(); | ||
| foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ) ) as $id => $script_module ) { | ||
| foreach ( $this->get_dependencies( array_unique( $this->queue ) ) as $id => $script_module ) { | ||
| $imports[ $id ] = $this->get_src( $id ); | ||
| } | ||
| return array( 'imports' => $imports ); | ||
| @@ -359,13 +353,10 @@ private function get_import_map(): array { | ||
| * @return array<string, array> Script modules marked for enqueue, keyed by script module identifier. | ||
| */ | ||
| private function get_marked_for_enqueue(): array { | ||
| $enqueued = array(); | ||
| foreach ( $this->registered as $id => $script_module ) { | ||
| if ( true === $script_module['enqueue'] ) { | ||
| $enqueued[ $id ] = $script_module; | ||
| } | ||
| } | ||
| return $enqueued; | ||
| return wp_array_slice_assoc( | ||
| $this->registered, | ||
| $this->queue | ||
| ); | ||
| } | ||
| /** | ||
| @@ -457,7 +448,7 @@ private function get_src( string $id ): string { | ||
| */ | ||
| public function print_script_module_data(): void { | ||
| $modules = array(); | ||
| foreach ( array_keys( $this->get_marked_for_enqueue() ) as $id ) { | ||
| foreach ( array_unique( $this->queue ) as $id ) { | ||
| if ( '@wordpress/a11y' === $id ) { | ||
| $this->a11y_available = true; | ||
| } | ||
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -31,9 +31,21 @@ public function set_up() { | ||
| global $post_ID; | ||
| $post_ID = 1; | ||
| global $wp_scripts, $wp_styles; | ||
| $this->original_wp_scripts = $wp_scripts; | ||
| $this->original_wp_styles = $wp_styles; | ||
| $wp_scripts = null; | ||
| $wp_styles = null; | ||
| wp_scripts(); | ||
| wp_styles(); | ||
westonruter marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| public function tear_down() { | ||
| global $wp_scripts, $wp_styles; | ||
| $wp_scripts = $this->original_wp_scripts; | ||
| $wp_styles = $this->original_wp_styles; | ||
| /** @var WP_REST_Server $wp_rest_server */ | ||
| global $wp_rest_server; | ||
| $wp_rest_server = null; | ||
| @@ -42,6 +54,16 @@ public function tear_down() { | ||
| parent::tear_down(); | ||
| } | ||
| /** | ||
| * @var WP_Scripts|null | ||
| */ | ||
| protected $original_wp_scripts; | ||
| /** | ||
| * @var WP_Styles|null | ||
| */ | ||
| protected $original_wp_styles; | ||
| public function filter_set_block_categories_post( $block_categories, $post ) { | ||
| if ( empty( $post ) ) { | ||
| return $block_categories; | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
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_queuemethod would be more helpful that requiring third party devs do anarray_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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This brings
WP_Script_Modulesin alignment withWP_ScriptsandWP_Styleswhich both expose a$queuemember variable. It allows us to easily obtain the list of IDs enqueued, and then merge them.For
WP_ScriptsandWP_Stylesit 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 becauseWP_Scripts,WP_Styles, andWP_Script_Modulesall obtain the unique items when processing anyway (or else keep track viadone).We could introduce a magic setter function for the
queuemember (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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, let's go with consistency.