-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Introduce fetchpriority
for Scripts and Script Modules
#8815
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
dbcd32c
5207c27
088366d
9eab85d
d9c4037
4f82a4f
f2cd9be
ff4fd36
9edbe4c
021fe74
34ef7fa
e54274a
cc1d909
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,11 +175,21 @@ function register_block_script_module_id( $metadata, $field_name, $index = 0 ) { | |
$block_version = isset( $metadata['version'] ) ? $metadata['version'] : false; | ||
$module_version = isset( $module_asset['version'] ) ? $module_asset['version'] : $block_version; | ||
|
||
$args = array(); | ||
if ( | ||
( isset( $metadata['supports']['interactivity'] ) && true === $metadata['supports']['interactivity'] ) || | ||
( isset( $metadata['supports']['interactivity']['interactive'] ) && true === $metadata['supports']['interactivity']['interactive'] ) | ||
) { | ||
// TODO: Add ability for the fetchpriority to be specified in block.json for the viewScriptModule. In wp_default_script_modules() the fetchpriority defaults to low since server-side rendering is employed for core blocks, but there are no guarantees that this is the case for non-core blocks. That said, viewScriptModule entails Interactivity API, following the pattern from core it _should_ be SSR'ed and that is why this is the default for when the block supports interactivity. | ||
$args['fetchpriority'] = 'low'; | ||
} | ||
|
||
wp_register_script_module( | ||
$module_id, | ||
$module_uri, | ||
$module_dependencies, | ||
$module_version | ||
$module_version, | ||
$args | ||
); | ||
|
||
return $module_id; | ||
|
@@ -243,6 +253,7 @@ function register_block_script_handle( $metadata, $field_name, $index = 0 ) { | |
$script_args = array(); | ||
if ( 'viewScript' === $field_name && $script_uri ) { | ||
$script_args['strategy'] = 'defer'; | ||
// TODO: There needs to be a way to specify that a script defined in a module is safe to use fetchpriority=low. Perhaps the viewScript should not only allow a handle string or a `file:./foo.js` string, but allow an an array of params like { "href": "./foo.js", "fetchpriority": "low" }. This would allow for more metadata to be supplied, like the script loading strategy. Related: <https://core.trac.wordpress.org/ticket/56408> and <https://core.trac.wordpress.org/ticket/54018>. | ||
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. Oh, yes, exactly what I meant above 😅 |
||
} | ||
|
||
$result = wp_register_script( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,13 +12,22 @@ | |
* Core class used to register script modules. | ||
* | ||
* @since 6.5.0 | ||
* | ||
* @phpstan-type ScriptModule array{ | ||
* src: string, | ||
* version: string|false|null, | ||
* enqueue: bool, | ||
* dependencies: array<array{ id: string, import: 'dynamic'|'static' }>, | ||
* fetchpriority: 'auto'|'low'|'high', | ||
* } | ||
Comment on lines
+16
to
+22
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. I added these |
||
*/ | ||
class WP_Script_Modules { | ||
/** | ||
* Holds the registered script modules, keyed by script module identifier. | ||
* | ||
* @since 6.5.0 | ||
* @var array[] | ||
* @phpstan-var array<string, ScriptModule> | ||
*/ | ||
private $registered = array(); | ||
|
||
|
@@ -46,6 +55,7 @@ class WP_Script_Modules { | |
* identifier has already been registered. | ||
* | ||
* @since 6.5.0 | ||
* @since n.e.x.t Added the $args parameter. | ||
* | ||
* @param string $id The identifier of the script module. Should be unique. It will be used in the | ||
* final import map. | ||
|
@@ -71,13 +81,18 @@ class WP_Script_Modules { | |
* It is added to the URL as a query string for cache busting purposes. If $version | ||
* is set to false, the version number is the currently installed WordPress version. | ||
* If $version is set to null, no version is added. | ||
* @param array $args { | ||
* Optional. An array of additional args. Default empty array. | ||
* | ||
* @type 'auto'|'low'|'high' $fetchpriority Fetch priority. Default 'auto'. Optional. | ||
* } | ||
*/ | ||
public function register( string $id, string $src, array $deps = array(), $version = false ) { | ||
public function register( string $id, string $src, array $deps = array(), $version = false, array $args = array() ) { | ||
if ( ! isset( $this->registered[ $id ] ) ) { | ||
$dependencies = array(); | ||
foreach ( $deps as $dependency ) { | ||
if ( is_array( $dependency ) ) { | ||
if ( ! isset( $dependency['id'] ) ) { | ||
if ( ! isset( $dependency['id'] ) || ! is_string( $dependency['id'] ) ) { | ||
_doing_it_wrong( __METHOD__, __( 'Missing required id key in entry among dependencies array.' ), '6.5.0' ); | ||
continue; | ||
} | ||
|
@@ -95,22 +110,76 @@ public function register( string $id, string $src, array $deps = array(), $versi | |
} | ||
} | ||
|
||
$fetchpriority = 'auto'; | ||
if ( isset( $args['fetchpriority'] ) ) { | ||
if ( $this->is_valid_fetchpriority( $args['fetchpriority'] ) ) { | ||
$fetchpriority = $args['fetchpriority']; | ||
} else { | ||
_doing_it_wrong( | ||
__METHOD__, | ||
/* translators: %s: Invalid fetchpriority. */ | ||
sprintf( __( 'Invalid fetchpriority: %s' ), $args['fetchpriority'] ), | ||
'n.e.x.t' | ||
); | ||
} | ||
} | ||
|
||
$this->registered[ $id ] = array( | ||
'src' => $src, | ||
'version' => $version, | ||
'enqueue' => isset( $this->enqueued_before_registered[ $id ] ), | ||
'dependencies' => $dependencies, | ||
'src' => $src, | ||
'version' => $version, | ||
'enqueue' => isset( $this->enqueued_before_registered[ $id ] ), | ||
'dependencies' => $dependencies, | ||
'fetchpriority' => $fetchpriority, | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the provided fetchpriority is valid. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @param mixed $priority Fetch priority. | ||
* @return bool Whether valid fetchpriority. | ||
*/ | ||
private function is_valid_fetchpriority( $priority ): bool { | ||
return in_array( $priority, array( 'auto', 'low', 'high' ), true ); | ||
} | ||
|
||
/** | ||
* Sets the fetch priority for a script module. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @param string $id Script module identifier. | ||
* @param 'auto'|'low'|'high' $priority Fetch priority for the script module. | ||
*/ | ||
public function set_fetchpriority( string $id, string $priority ) { | ||
if ( ! isset( $this->registered[ $id ] ) ) { | ||
return; | ||
} | ||
|
||
if ( ! $this->is_valid_fetchpriority( $priority ) ) { | ||
_doing_it_wrong( | ||
__METHOD__, | ||
/* translators: %s: Invalid fetchpriority. */ | ||
sprintf( __( 'Invalid fetchpriority: %s' ), $priority ), | ||
'n.e.x.t' | ||
); | ||
return; | ||
} | ||
|
||
$this->registered[ $id ]['fetchpriority'] = $priority; | ||
} | ||
|
||
/** | ||
* Marks the script module to be enqueued in the page. | ||
* | ||
* If a src is provided and the script module has not been registered yet, it | ||
* will be registered. | ||
* | ||
* @since 6.5.0 | ||
* @since n.e.x.t Added the $args parameter. | ||
* | ||
* @param string $id The identifier of the script module. Should be unique. It will be used in the | ||
* final import map. | ||
|
@@ -136,12 +205,17 @@ public function register( string $id, string $src, array $deps = array(), $versi | |
* It is added to the URL as a query string for cache busting purposes. If $version | ||
* is set to false, the version number is the currently installed WordPress version. | ||
* If $version is set to null, no version is added. | ||
* @param array $args { | ||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Optional. An array of additional args. Default empty array. | ||
* | ||
* @type 'auto'|'low'|'high' $fetchpriority Fetch priority. Default 'auto'. Optional. | ||
* } | ||
*/ | ||
public function enqueue( string $id, string $src = '', array $deps = array(), $version = false ) { | ||
public function enqueue( string $id, string $src = '', array $deps = array(), $version = false, $args = array() ) { | ||
if ( isset( $this->registered[ $id ] ) ) { | ||
$this->registered[ $id ]['enqueue'] = true; | ||
} elseif ( $src ) { | ||
$this->register( $id, $src, $deps, $version ); | ||
$this->register( $id, $src, $deps, $version, $args ); | ||
$this->registered[ $id ]['enqueue'] = true; | ||
} else { | ||
$this->enqueued_before_registered[ $id ] = true; | ||
|
@@ -208,13 +282,15 @@ public function add_hooks() { | |
*/ | ||
public function print_enqueued_script_modules() { | ||
foreach ( $this->get_marked_for_enqueue() as $id => $script_module ) { | ||
wp_print_script_tag( | ||
array( | ||
'type' => 'module', | ||
'src' => $this->get_src( $id ), | ||
'id' => $id . '-js-module', | ||
) | ||
$args = array( | ||
'type' => 'module', | ||
'src' => $this->get_src( $id ), | ||
'id' => $id . '-js-module', | ||
); | ||
if ( 'auto' !== $script_module['fetchpriority'] ) { | ||
$args['fetchpriority'] = $script_module['fetchpriority']; | ||
} | ||
wp_print_script_tag( $args ); | ||
} | ||
} | ||
|
||
|
@@ -231,9 +307,10 @@ public function print_script_module_preloads() { | |
// Don't preload if it's marked for enqueue. | ||
if ( true !== $script_module['enqueue'] ) { | ||
echo sprintf( | ||
'<link rel="modulepreload" href="%s" id="%s">', | ||
'<link rel="modulepreload" href="%s" id="%s"%s>', | ||
esc_url( $this->get_src( $id ) ), | ||
esc_attr( $id . '-js-modulepreload' ) | ||
esc_attr( $id . '-js-modulepreload' ), | ||
'auto' !== $script_module['fetchpriority'] ? sprintf( ' fetchpriority="%s"', esc_attr( $script_module['fetchpriority'] ) ) : '' | ||
); | ||
} | ||
} | ||
|
@@ -278,7 +355,9 @@ private function get_import_map(): array { | |
* | ||
* @since 6.5.0 | ||
* | ||
* @return array[] Script modules marked for enqueue, keyed by script module identifier. | ||
* @phpstan-return array<string, ScriptModule> | ||
* | ||
* @return array<string, array> Script modules marked for enqueue, keyed by script module identifier. | ||
*/ | ||
private function get_marked_for_enqueue(): array { | ||
$enqueued = array(); | ||
|
@@ -300,6 +379,8 @@ private function get_marked_for_enqueue(): array { | |
* | ||
* @since 6.5.0 | ||
* | ||
* @phpstan-return array<string, ScriptModule> | ||
* | ||
* @param string[] $ids The identifiers of the script modules for which to gather dependencies. | ||
* @param string[] $import_types Optional. Import types of dependencies to retrieve: 'static', 'dynamic', or both. | ||
* Default is both. | ||
|
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.
Maybe that property in
block.json
should become an object instead of a string. In the future we might need other properties in there too.cc @gziolo
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.
Absolutely! See below 😄 #8815 (comment)