-
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
Changes from all commits
69fe993
e2650be
e2828a7
6c38110
3cc250e
2e10eb3
1ab3c34
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 |
|---|---|---|
|
|
@@ -653,8 +653,23 @@ function gutenberg_default_script_modules() { | |
| break; | ||
| } | ||
|
|
||
| /* | ||
| * All script modules in Gutenberg are (currently) related to the Interactivity API which prioritizes server-side rendering. | ||
| * Therefore, the modules should be fetched with a low priority to avoid network contention with any LCP element resource. | ||
| * For allowing a block to opt-in to another fetchpriority, see <https://github.com/WordPress/gutenberg/issues/71366>. | ||
| * | ||
| * Also, the @wordpress/a11y script module is intended to be used as a dynamic import dependency, in which case | ||
| * the fetchpriority is irrelevant. See <https://make.wordpress.org/core/2024/10/14/updates-to-script-modules-in-6-7/>. | ||
| * However, in case it is added as a static import dependency, the fetchpriority is explicitly set to be 'low' | ||
| * since the module should not be involved in the critical rendering path, and if it is, its fetchpriority will | ||
| * be bumped to match the fetchpriority of the dependent script. | ||
| */ | ||
| $args = array( | ||
| 'fetchpriority' => 'low', | ||
| ); | ||
|
|
||
| $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>. | ||
|
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. Nice!
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. I checked to make sure extra arguments to functions don't cause errors/warnings/notices and that does seem to be the case 👍
Member
Author
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. 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 |
||
| } | ||
| } | ||
| remove_action( 'wp_default_scripts', 'wp_default_script_modules' ); | ||
|
|
||
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/a11ymodule 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
a11ybelowas 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 theimportmapand so there is nofetchpriorityto 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 Coretrunkthen it omitsfetchpriority(so it isauto).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