CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add fetchpriority=low support to script modules #70173
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
Conversation
Flaky tests detected in 1ab3c34. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17508853441
|
…dd/script-module-fetchpriority-low
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
* Allow scripts and script modules to be registered with a `fetchpriority` of `auto` (default), `high`, `low`: * When registering a script, add a `fetchpriority` arg to go alongside the `strategy` arg which was added for loading scripts with the `defer` and `async` loading strategies. See #12009. * For script modules, introduce an `$args` array parameter with a `fetchpriority` key to the `wp_register_script_module()`, and `wp_enqueue_script_module()` functions (and their respective underlying `WP_Script_Modules::register()` and `WP_Script_Modules::enqueue()` methods). This `$args` parameter corresponds with the same parameter used when registering non-module scripts. * Also for script modules, introduce `WP_Script_Modules::set_fetchpriority()` to override the `fetchpriority` for what was previously registered. * Emit a `_doing_it_wrong()` warning when an invalid `fetchpriority` value is used, and when `fetchpriority` is added to a script alias. * Include `fetchpriority` as an attribute on printed `SCRIPT` tags as well as on preload `LINK` tags for static script module dependencies. * Use a `fetchpriority` of `low` by default for: * Script modules used with the Interactivity API. For overriding this default in blocks, see [WordPress/gutenberg#71366 Gutenberg#71366]. * The `comment-reply` script. * Improve type checks and type hints. Developed in [#8815 GitHub PR], with [WordPress/gutenberg#70173 companion for Gutenberg]. Props westonruter, jonsurrell, swissspidy, luisherranz, kraftbj, audrasjb, dennysdionigi. Fixes #61734. git-svn-id: https://develop.svn.wordpress.org/trunk@60704 602fd350-edb4-49c9-b593-d223f7449a82
* Allow scripts and script modules to be registered with a `fetchpriority` of `auto` (default), `high`, `low`: * When registering a script, add a `fetchpriority` arg to go alongside the `strategy` arg which was added for loading scripts with the `defer` and `async` loading strategies. See #12009. * For script modules, introduce an `$args` array parameter with a `fetchpriority` key to the `wp_register_script_module()`, and `wp_enqueue_script_module()` functions (and their respective underlying `WP_Script_Modules::register()` and `WP_Script_Modules::enqueue()` methods). This `$args` parameter corresponds with the same parameter used when registering non-module scripts. * Also for script modules, introduce `WP_Script_Modules::set_fetchpriority()` to override the `fetchpriority` for what was previously registered. * Emit a `_doing_it_wrong()` warning when an invalid `fetchpriority` value is used, and when `fetchpriority` is added to a script alias. * Include `fetchpriority` as an attribute on printed `SCRIPT` tags as well as on preload `LINK` tags for static script module dependencies. * Use a `fetchpriority` of `low` by default for: * Script modules used with the Interactivity API. For overriding this default in blocks, see [WordPress/gutenberg#71366 Gutenberg#71366]. * The `comment-reply` script. * Improve type checks and type hints. Developed in [WordPress/wordpress-develop#8815 GitHub PR], with [WordPress/gutenberg#70173 companion for Gutenberg]. Props westonruter, jonsurrell, swissspidy, luisherranz, kraftbj, audrasjb, dennysdionigi. Fixes #61734. Built from https://develop.svn.wordpress.org/trunk@60704 git-svn-id: https://core.svn.wordpress.org/trunk@60040 1a063a9b-81f0-0310-95a4-ce76da25c4cd
* Allow scripts and script modules to be registered with a `fetchpriority` of `auto` (default), `high`, `low`: * When registering a script, add a `fetchpriority` arg to go alongside the `strategy` arg which was added for loading scripts with the `defer` and `async` loading strategies. See #12009. * For script modules, introduce an `$args` array parameter with a `fetchpriority` key to the `wp_register_script_module()`, and `wp_enqueue_script_module()` functions (and their respective underlying `WP_Script_Modules::register()` and `WP_Script_Modules::enqueue()` methods). This `$args` parameter corresponds with the same parameter used when registering non-module scripts. * Also for script modules, introduce `WP_Script_Modules::set_fetchpriority()` to override the `fetchpriority` for what was previously registered. * Emit a `_doing_it_wrong()` warning when an invalid `fetchpriority` value is used, and when `fetchpriority` is added to a script alias. * Include `fetchpriority` as an attribute on printed `SCRIPT` tags as well as on preload `LINK` tags for static script module dependencies. * Use a `fetchpriority` of `low` by default for: * Script modules used with the Interactivity API. For overriding this default in blocks, see [WordPress/gutenberg#71366 Gutenberg#71366]. * The `comment-reply` script. * Improve type checks and type hints. Developed in [WordPress/wordpress-develop#8815 GitHub PR], with [WordPress/gutenberg#70173 companion for Gutenberg]. Props westonruter, jonsurrell, swissspidy, luisherranz, kraftbj, audrasjb, dennysdionigi. Fixes #61734. Built from https://develop.svn.wordpress.org/trunk@60704 git-svn-id: https://core.svn.wordpress.org/trunk@60040 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
||
$path = gutenberg_url( "build-module/{$file_name}" ); | ||
wp_register_script_module( $script_module_id, $path, $script_module_data['dependencies'], $script_module_data['version'] ); | ||
wp_register_script_module( $script_module_id, $path, $script_module_data['dependencies'], $script_module_data['version'], $args ); // The $args parameter is new as of WP 6.9 per <https://core.trac.wordpress.org/ticket/61734>. |
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.
Nice!
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.
I checked to make sure extra arguments to functions don't cause errors/warnings/notices and that does seem to be the case 👍
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.
Yeah, adding extra arguments is fine. No complaints going back even to PHP 4.3: https://3v4l.org/doYun
Any additional unnamed params could be obtained by the function via func_get_args()
.
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
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.
Two things come to mind seeing this:
Script modules are registered with fetch priority. What if I enqueue a high
fetch priority script that depends on a module registered with low
? As the browser explores the dependency graph, will it determine that it should increase the priority of the dependency? Is that sort of priority dependence something that should be built into the core script modules system?
This makes all the Gutenberg script modules low
. That's fine at the moment. Have you considered how this system may evolve as more modules are introduced? What mechanism might modules have to report what fetch priority they should have?
|
||
$path = gutenberg_url( "build-module/{$file_name}" ); | ||
wp_register_script_module( $script_module_id, $path, $script_module_data['dependencies'], $script_module_data['version'] ); | ||
wp_register_script_module( $script_module_id, $path, $script_module_data['dependencies'], $script_module_data['version'], $args ); // The $args parameter is new as of WP 6.9 per <https://core.trac.wordpress.org/ticket/61734>. |
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.
I checked to make sure extra arguments to functions don't cause errors/warnings/notices and that does seem to be the case 👍
} | ||
|
||
/* | ||
* All script modules in Gutenberg are (currently) related to the Interactivity API which prioritizes server-side rendering. |
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.
There's @wordpress/a11y
module that's not strictly an iAPI concept.
https://make.wordpress.org/core/2024/10/14/updates-to-script-modules-in-6-7/
I don't think other modules have been introduced, but I could have missed something. Should a11y
be low
as well?
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.
Strictly, but it is only used by the interactivity-router
, right? And it is used as a dynamic import, so it exists exclusively in the importmap
and so there is no fetchpriority
to print.
Granted, it could end up getting added as a static dependency. If I try doing that, I see that with this Gutenberg PR it is printed with fetchpriority=low
, but if I try WP Core trunk
then it omits fetchpriority
(so it is auto
).
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.
I've addressed this in 1ab3c34.
And in core, I've addressed in WordPress/wordpress-develop#9770 via WordPress/wordpress-develop@e2d4676
…dd/script-module-fetchpriority-low
That's a great question. I put together a try-script-module-priorities plugin to test this. The plugin registers three modules, If <script
type="module"
src="https://localhost:8000/wp-content/plugins/try-script-module-priorities/baz.js"
id="baz-js-module"
fetchpriority="high"
></script>
<link
rel="modulepreload"
href="https://localhost:8000/wp-content/plugins/try-script-module-priorities/bar.js"
id="bar-js-modulepreload"
fetchpriority="low"
/>
<link
rel="modulepreload"
href="https://localhost:8000/wp-content/plugins/try-script-module-priorities/foo.js"
id="foo-js-modulepreload"
fetchpriority="low"
/> The result seems to be that the dependencies are not fetched with a high priority: ![]() I see the same in Firefox and Safari. It would be ideal that if a script module had a
For |
|
Co-authored-by: Jon Surrell <jonsurrell@git.wordpress.org>
* Add fetchpriority=low support to script modules * Update comment now that change landed in core * Add reference to related GB issue * Use multi-line comment Co-authored-by: westonruter <westonruter@git.wordpress.org> Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com> * Add comment explaining why a11y is fetchpriority=low Co-authored-by: Jon Surrell <jonsurrell@git.wordpress.org> --------- Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com> Co-authored-by: Jon Surrell <jonsurrell@git.wordpress.org>
* Allow scripts and script modules to be registered with a `fetchpriority` of `auto` (default), `high`, `low`: * When registering a script, add a `fetchpriority` arg to go alongside the `strategy` arg which was added for loading scripts with the `defer` and `async` loading strategies. See #12009. * For script modules, introduce an `$args` array parameter with a `fetchpriority` key to the `wp_register_script_module()`, and `wp_enqueue_script_module()` functions (and their respective underlying `WP_Script_Modules::register()` and `WP_Script_Modules::enqueue()` methods). This `$args` parameter corresponds with the same parameter used when registering non-module scripts. * Also for script modules, introduce `WP_Script_Modules::set_fetchpriority()` to override the `fetchpriority` for what was previously registered. * Emit a `_doing_it_wrong()` warning when an invalid `fetchpriority` value is used, and when `fetchpriority` is added to a script alias. * Include `fetchpriority` as an attribute on printed `SCRIPT` tags as well as on preload `LINK` tags for static script module dependencies. * Use a `fetchpriority` of `low` by default for: * Script modules used with the Interactivity API. For overriding this default in blocks, see [WordPress/gutenberg#71366 Gutenberg#71366]. * The `comment-reply` script. * Improve type checks and type hints. Developed in [WordPress#8815 GitHub PR], with [WordPress/gutenberg#70173 companion for Gutenberg]. Props westonruter, jonsurrell, swissspidy, luisherranz, kraftbj, audrasjb, dennysdionigi. Fixes #61734. git-svn-id: https://develop.svn.wordpress.org/trunk@60704 602fd350-edb4-49c9-b593-d223f7449a82
This is to add support for the new
$args
parameter forwp_register_script_module()
/wp_enqueue_script_module()
for Core-61734 and proposed in WordPress/wordpress-develop#8815. When Gutenberg is active, thefetchpriority=low
won't appear for script modules since the core script modules are overridden by Gutenberg.