Make WordPress Core

Opened 3 months ago

Last modified 11 days ago

#61734 assigned enhancement

Add the ability to handle "fetchpriority" to ES Modules and Import Maps

Reported by: dennysdionigi's profile dennysdionigi Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 6.5
Component: Script Loader Keywords: needs-patch 2nd-opinion
Focuses: javascript, performance Cc:

Description

This is a follow-up to #56313.

Following this ticket, seems that adding fetchpriority to module scripts gives no luck.

So any imported script has high priority, even when they should have low fetching prioritize.


I guess that should be a plus, to allow that choise / filter also for modules and importmaps, while actually loader_tag , script_add_data, not even replace or HTML API seem having effect.

I guess that giving a low priority hint, sparingly and with proper controls, can bring an improvement in overall performance.

Reference documentation: https://developer.mozilla.org/en-US/docs/Web/API/HTMLScriptElement/fetchPriority

Change History (14)

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 months ago

#2 @westonruter
3 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.7
  • Owner set to westonruter
  • Status changed from new to accepted
  • Version changed from 6.6.1 to 6.5

I do indeed see that on a page with the Interactivity API present for the Image block and the Navigation block, that these scripts are all loaded with high priority in Chrome:

  • /wp-includes/blocks/navigation/view.js
  • /wp-includes/blocks/image/view.js
  • /wp-includes/js/dist/interactivity.js

The HTML in question:

<script type="importmap" id="wp-importmap">
{"imports":{"@wordpress\/interactivity":"http:\/\/localhost:8888\/wp-includes\/js\/dist\/interactivity.js?ver=6.7-alpha-58835"}}
</script>
<script type="module" src="http://localhost:8888/wp-includes/blocks/navigation/view.js?ver=6.7-alpha-58835" id="@wordpress/block-library/navigation-js-module"></script>
<script type="module" src="http://localhost:8888/wp-includes/blocks/image/view.js?ver=6.7-alpha-58835" id="@wordpress/block-library/image-js-module"></script>
<link rel="modulepreload" href="http://localhost:8888/wp-includes/js/dist/interactivity.js?ver=6.7-alpha-58835" id="@wordpress/interactivity-js-modulepreload">

This doesn't seem ideal, considering that server-side rendering should be employed by interactive blocks, meaning that the scripting should not be in the critical path for rendering the page. By loading these scripts with high priority, they may be hurting LCP by adding network contention for fetching the LCP image.

For adding fetchpriority for a script module, either this could be added to the script tag itself or it could be added as a link[rel=modulepreload] tag that the same src.

For traditional scripts and module scripts alike, you can use the wp_script_attributes filter to inject the fetchpriority attribute onto the script tag. For example, to mark all module scripts to load with fetchpriority=low:

<?php
add_filter(
        'wp_script_attributes',
        static function ( $attributes ) {
                if (
                        isset( $attributes['type'], $attributes['id'] ) &&
                        'module' === $attributes['type'] &&
                        str_starts_with( $attributes['id'], '@wordpress/block-library/' )
                ) {
                        $attributes['fetchpriority'] = 'low';
                }
                return $attributes;
        }
);

However, this does not help when a script module is not directly enqueued in which case a link[rel=modulepreload] is added via WP_Script_Modules::print_script_module_preloads() which normally occurs for the @wordpress/interactivity script. And there is no way to insert fetchpriority=low into the generated link tag without some hacking:

<?php
add_action(
        'init',
        static function () {
                $position = wp_is_block_theme() ? 'wp_head' : 'wp_footer';
                $priority = has_action( $position, array( wp_script_modules(), 'print_script_module_preloads' ) );
                if ( false === $priority ) {
                        return;
                }
                remove_action( $position, array( wp_script_modules(), 'print_script_module_preloads' ), $priority );
                add_action(
                        $position,
                        static function () {
                                ob_start();
                                wp_script_modules()->print_script_module_preloads();
                                $buffer = ob_get_clean();
                                if ( '' === $buffer ) {
                                        return;
                                }
                                $processor = new WP_HTML_Tag_Processor( $buffer );
                                while ( $processor->next_tag( array( 'tag_name' => 'LINK' ) ) ) {
                                        $processor->set_attribute( 'fetchpriority', 'low' );
                                }
                                echo $processor->get_updated_html();
                        },
                        $priority
                );
        }
);

I put these snippets in a plugin you can use for testing as well: https://gist.github.com/westonruter/471111a891f43e0f48bc7e0ca478623d

Given a test post using the TT4 theme that contains one lightbox-enabled Image block which is the LCP element, where the responsive Navigation block is also present:

I tried benchmarking page loads with the existing behavior where all scripts are loaded with high priority, and over 100 requests the median LCP-TTFB was 48.15 ms.

Then I tried adding fetchpriority=low to the scripts, and over 100 requests the median LCP-TTFB was 44.6 ms.

So adding fetchpriority=low makes the LCP metric here 7.37% faster.

(The TTFB-LCP metric here is the LCP discounting variations in TTFB.)

I think the Interactivity API should add fetchpriority=low to all of the module scripts it registers.

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


3 months ago

#4 follow-up: @westonruter
2 months ago

I'm having a hard time reproducing the results I saw above. Now I'm seeing the addition of fetchpriority=low to be hurting LCP when the LCP element is a large Image block. This doesn't make sense to me.

Last edited 2 months ago by westonruter (previous) (diff)

#5 in reply to: ↑ 4 @dennysdionigi
2 months ago

Replying to westonruter:

I'm having a hard time reproducing the results I saw above. Now I'm seeing the addition of fetchpriority=low to be hurting LCP when the LCP element is a large Image block. This doesn't make sense to me.


First LCP image should have always fetchpriority high, the perf hint combo:

  • Heavy non essential resources: imgs, videos, links, scripts (example heavy animation scripts) low priority
  • imgs, iframe and videos can/should be further optimized by lazy-loading, decoding async - something WP already someway does...
  • Heavy essential resources (like lcp image), hypothetically also fonts, and primary static content, can get high priority hint, with additionally extra links prefetch/preconnect/preload - or newest speculation api for scripts.

  • fonts than can have an additionally optimisation trick: using a font-display optional (with no swap timing, with no-blocking timing), and a link rel preload to that font, gives a really huge performance boost, fixing some loading issues; but this is more like a "pro-hack"; font-display fallback, usually is enough. 😅

Otherwise adding wrong priority, or adding it to everything can lead the opposite results, as you said for the lcp image...

#6 @westonruter
2 months ago

@dennysdionigi yes, I'm well familiar, which is why I'm confused why I'm not seeing that loading the modules with low priority is not improving LCP on a page that has an image with fetchpriority=high. I'm sure I saw an improvement when I tested before. We need to put together a test page that shows consistent performance improvements.

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


8 weeks ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


7 weeks ago

#9 @mukesh27
7 weeks ago

  • Keywords 2nd-opinion added

This ticket was discussed in today's performance bug scrub. Someone needs to verify Weston's findings https://core.trac.wordpress.org/ticket/61734#comment:2 before any implementation.

Props to @joemcgill.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


5 weeks ago

#11 @westonruter
3 weeks ago

  • Owner westonruter deleted
  • Status changed from accepted to assigned

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 weeks ago

#13 @mukesh27
3 weeks ago

  • Milestone changed from 6.7 to Future Release

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


11 days ago

Note: See TracTickets for help on using tickets.