Make WordPress Core

Opened 6 years ago

Closed 21 months ago

Last modified 18 months ago

#42438 closed enhancement (fixed)

Add support for preload links (similar to resource hints)

Reported by: nico23's profile nico23 Owned by: swissspidy's profile swissspidy
Milestone: 6.1 Priority: normal
Severity: normal Version: 4.9
Component: Script Loader Keywords: has-unit-tests has-patch commit needs-dev-note
Focuses: performance Cc:

Description

<link rel="preload" ...

is currently not possible, while others like prefetch are.

Attachments (5)

add_preload_to_ressource_hints.patch (716 bytes) - added by nico23 6 years ago.
patch
42438.diff (13.6 KB) - added by adamsilverstein 21 months ago.
42438.2.diff (13.7 KB) - added by adamsilverstein 21 months ago.
42438.3.diff (13.8 KB) - added by adamsilverstein 21 months ago.
42438-function.diff (3.6 KB) - added by adamsilverstein 20 months ago.

Download all attachments as: .zip

Change History (76)

#1 @swissspidy
6 years ago

  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.0
  • Owner set to swissspidy
  • Status changed from new to assigned

Previously: #34292, #38121

Browser support looks much better now: https://caniuse.com/#feat=link-rel-preload

#2 @swissspidy
6 years ago

  • Component changed from General to Script Loader

#3 @igrigorik
6 years ago

Hey folks! Would love to see preload available in core. A couple of drive-by thoughts..

Resource Hints [1] and Preload [2] have different semantics; Resource Hints != Preload.

  • dns-prefect, preconnect, prefetch, prerender are speculative optimization hints
  • preload is a directive to the UA to fetch the resource

Addy Osmani has a nice writeup on the differences [3]. My recommendation would be to keep Preload and RH as separate APIs, because they have different processing behavior and attributes. Case in point, current wp_resource_hints() exposes "as" attribute, but that only applies to rel=preload resources. On a related note "pr" attribute is now deprecated, so that can be removed from the function as well.

Concretely, I'd propose...

  1. Do some cleanup on wp_resource_hints() to remove "pr" and "as".
  2. Expose new wp_preload() function to emit preload links.

WDYT? Crazytalk? :)

---
[1] https://w3c.github.io/resource-hints/
[2] https://w3c.github.io/preload/
[3] https://medium.com/reloading/preload-prefetch-and-priorities-in-chrome-776165961bbf

#4 @swissspidy
6 years ago

  • Keywords needs-patch added; has-patch removed

#5 follow-up: @westonruter
6 years ago

I just ran across this as I was looking to add <link rel="preload" as="script" href="https://cdn.ampproject.org/v0.js"> to the AMP plugin.

While preload != resource hints it seems the existing resource hints API in WordPress is well suited for preload. For example, with add_preload_to_ressource_hints.patch:

<?php
add_filter( 'wp_resource_hints', function( $urls, $relation_type ) {
        if ( 'preload' === $relation_type ) {
                $urls[] = array(
                        'rel'  => 'preload',
                        'as'   => 'script',
                        'href' => 'https://cdn.ampproject.org/v0.js',
                );
        }
        return $urls;
}, 10, 2 );

@igrigorik Would this be bad?

As for the pr attribute in wp_resource_hints(), it's just allowing for the attribute to be used, it doesn't mandate it. We could add a special condition to _doing_it_wrong() if that attribute is provided. Likewise, we could call _doing_it_wrong() if the as attribute is used with anything other than rel=preload.

Also, does it matter the order of the links? Should preload links come after resource hint links?

#6 in reply to: ↑ 5 ; follow-up: @igrigorik
6 years ago

Replying to westonruter:

I just ran across this as I was looking to add <link rel="preload" as="script" href="https://cdn.ampproject.org/v0.js"> to the AMP plugin.

While preload != resource hints it seems the existing resource hints API in WordPress is well suited for preload. For example, with add_preload_to_ressource_hints.patch:

<?php
add_filter( 'wp_resource_hints', function( $urls, $relation_type ) {
        if ( 'preload' === $relation_type ) {
                $urls[] = array(
                        'rel'  => 'preload',
                        'as'   => 'script',
                        'href' => 'https://cdn.ampproject.org/v0.js',
                );
        }
        return $urls;
}, 10, 2 );

@igrigorik Would this be bad?

Not sure if "bad" is the right term, but at a risk of being pedantic.. personally I _would_ prefer if preload was exposed as standalone and separate API: "hints" have a different semantic and UA behavior from preload and merging them into the same API might cause unnecessary confusion for a bunch of folks.

If I could make a wish, I think it would be:

  • deprecate as and pr attributes on wp_resource_hints
  • introduce wp_preload with support for as *and* enforce that as must be provided

Splitting these into separate top level APIs matches the spec behavior and intent, and I think it would help developers navigate the requirements much more clearly too.

As for the pr attribute in wp_resource_hints(), it's just allowing for the attribute to be used, it doesn't mandate it. We could add a special condition to _doing_it_wrong() if that attribute is provided. Likewise, we could call _doing_it_wrong() if the as attribute is used with anything other than rel=preload.

Yeah, I think could work. This wouldn't be my first choice from overall design perspective, but it would get the job done.

Q: Is there a much higher lift to implement and ship separate wp_preload API?

Also, does it matter the order of the links? Should preload links come after resource hint links?

No, the order does not matter.

#7 in reply to: ↑ 6 ; follow-up: @westonruter
6 years ago

Replying to igrigorik:

Not sure if "bad" is the right term, but at a risk of being pedantic.. personally I _would_ prefer if preload was exposed as standalone and separate API: "hints" have a different semantic and UA behavior from preload and merging them into the same API might cause unnecessary confusion for a bunch of folks.

If I could make a wish, I think it would be:

  • deprecate as and pr attributes on wp_resource_hints
  • introduce wp_preload with support for as *and* enforce that as must be provided

Splitting these into separate top level APIs matches the spec behavior and intent, and I think it would help developers navigate the requirements much more clearly too.

That sounds good. I think wp_preload_links would be marginally better name for this function/filter. A benefit to having a separate function as you proposed is that it simplifies the filter. Instead of as above having to check 'preload' === $relation_type the filter could be instead added simply as:

<?php
add_filter( 'wp_preload_links', function( $urls ) {
        $urls[] = array(
                'as'   => 'script',
                'href' => 'https://cdn.ampproject.org/v0.js',
        );
        return $urls;
} );

Aside: I've opened a PR to add the requisite preload links for the AMP plugin via the post-processor. This works in the AMP plugin because we have the output loaded in a DOMDocument for manipulation which isn't an approach available in WordPress normally.

#8 @westonruter
6 years ago

Stepping back a bit: what in WordPress core today would benefit from preload links? We should have some good use cases in core (or bundled themes) prior to adding this.

#9 in reply to: ↑ 7 @igrigorik
6 years ago

Replying to westonruter:

I think wp_preload_links would be marginally better name for this function/filter. A benefit to having a separate function as you proposed is that it simplifies the filter.

Thumbs up for wp_preload_links. I'll defer on the shape of the WP function to the other experts here though. :-)

Stepping back a bit: what in WordPress core today would benefit from preload links? We should have some good use cases in core (or bundled themes) prior to adding this.

There's a lot of good content — e.g., see (1), (2), (3) — on various preload use cases that can be useful within core and themes. I'll just highlight a few...

  • early load of late discovered resources: fonts, stylesheets, hero images, media, etc.
  • decoupling fetch and execution: simple "async CSS" (with help of onload), manual scheduling of JS parse+execute, etc.
  • as a mechanism to signal H2 push to servers/CDNs

Not an exhaustive list, but I believe each of the above has definite use cases in WP ecosystem.


  1. https://developer.mozilla.org/en-US/docs/Web/HTML/Preloading_content
  2. https://www.smashingmagazine.com/2016/02/preload-what-is-it-good-for/
  3. https://developers.google.com/web/updates/2016/03/link-rel-preload

#10 @westonruter
6 years ago

@igrigorik of those use cases I think “early load of late discovered resources” is going to be the one that will make the most sense to start with in WordPress. It's continuing what was started in #37171 to add preconnect resource hint for the Google Fonts used in the core themes. Similar patches could be added to add support for preloading the header image and featured image that would be used on a given theme template.

#11 @westonruter
6 years ago

  • Summary changed from preload is missing from ressource hints. to Add support for preload links (similar to resource hints)

#12 @westonruter
6 years ago

For the preloading of images, this seems to be largely blocked by the implementation srcset and imgsizes attributes on link rel=preload in browsers. Since images in WordPress almost always get served with sizes and srcset, it doesn't seem feasible to preload the image since we don't know which image size will be used, and the browser could very well preload the wrong image and waste bandwidth. In fact, when I tried adding preloading for the header image in Twenty Seventeen, I got an warning in DevTools:

The resource … was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.

#13 @pento
6 years ago

  • Milestone changed from 5.0 to 5.1

#14 @swissspidy
5 years ago

  • Milestone changed from 5.1 to 5.2

#15 @westonruter
5 years ago

The imagesrcset and imagesizes attributes have landed in Chrome 73 beta: https://www.chromestatus.com/feature/5164259990306816

#16 @swissspidy
5 years ago

  • Milestone changed from 5.2 to 5.3

#17 @azaozz
5 years ago

  • Milestone changed from 5.3 to Future Release

Is add_preload_to_ressource_hints.patch something we still want in 5.3? Following the above discussion from about a year ago, seems preloading would need more?

Moving to future release for now, please feel free to bring back to 5.3 if feasible.

CC: @swissspidy

#18 @swissspidy
4 years ago

#49972 was marked as a duplicate.

This ticket was mentioned in PR #2505 on WordPress/wordpress-develop by manuelRod.


2 years ago
#19

  • Keywords has-patch added; needs-patch removed

#20 @furi3r
2 years ago

Hello folks, I've been working on a PR creating wp_preload_links ( covers imagesrcset and imagesizes too ), also added unit testing to it. cc @swissspidy @spacedmonkey

manuelRod commented on PR #2505:


2 years ago
#21

@swissspidy thanks for the feedback, I've addressed it. I also interacted on the performance issue, but I did not get any feedback from there.

#22 @furi3r
2 years ago

pinging others on here @westonruter @flixos90 for more feedback.

adamsilverstein commented on PR #2505:


2 years ago
#23

@manuelRod left some small feedback/questions on the PR

manuelRod commented on PR #2505:


2 years ago
#24

@adamsilverstein thanks for the feedback, I left also some answers

#25 @aristath
2 years ago

While exploring ways to improve performance for block themes in Core I came across this ticket again.
Block themes currently parse the content before the <head>, and then load a separate CSS file for each block-type that exists on a page. These files are tiny and most times get inlined, but not always (there's a filterable default maximum of 20kb for inlined styles).
When a block-style does not get inlined, we should make these styles available to the browser as soon as possible to prevent render-blocking. In order to do that, we need to allow preload and apply it to the core block stylesheets.

#26 @furi3r
2 years ago

Hello, @aristath thanks for your input. Do you mind going over the PR? any feedback is welcomed :)

manuelRod commented on PR #2505:


2 years ago
#27

@adamsilverstein @swissspidy @spacedmonkey all ready here

spacedmonkey commented on PR #2505:


2 years ago
#28

Please update on references to 6.0.0 and change them to 6.1.0.

spacedmonkey commented on PR #2505:


2 years ago
#29

This feels like it is very close.

#30 @spacedmonkey
2 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed
  • Milestone changed from Future Release to 6.1

The PR looks good in it's current form. I am going to add this to 6.1 milestone in hopes of getting it in.

manuelRod commented on PR #2505:


2 years ago
#31

@spacedmonkey references version bumped

#32 @mukesh27
23 months ago

The PR LGTM.

Can we move wp_preload_links action above the wp_resource_hints because it's priority is 1 and it will run before the wp_resource_hints action?

#33 @swissspidy
23 months ago

Are we confident the current API is intuitive enough and can be used to solve typical use cases? Having to use a filter to add a preload link vs. an imperative way (e.g. a wp_add_preload_link() function or so) is arguably not the best developer experience. It’s one of the things I am not super proud of from when I helped with the resource hints feature.

Already asked 4 years ago: what in WordPress core today would benefit from preload links? We should have some good use cases in core (or bundled themes) prior to adding this. There should be some examples that proof that the proposed API can solve these use cases.

#34 @furi3r
22 months ago

Thanks for your feedback @swissspidy.

Regarding filter vs imperative way, I agree with you that filter may not be the most intuitive way, but at this stage, and following the current wp_resource_hints, I would go down this path for now, and maybe in a separate ticket, create imperative functionality, I wouldn't like to see this ticket stale for another 5 years.

Regarding core functionality using this preload feature, I'm not sure about that, but I'm sure this will be helpful for theme/plugins/performance developers, IMHO I don't see why offering this extra possibility could hurt core, it's a nice benefit to have for whoever wants to use it.

#36 follow-up: @peterwilsoncc
22 months ago

I think using the existing code in wp_resource_hints would be better.

The pros of using a separate function is that it is semantically correct. The cons are that it duplicates a near identical API; it's not possible to coordinate between preload and hinting to avoid double ups.

If anyone feels strongly enough that there is a better name that can encompass both hinting and instructing the browser, the the existing name can be deprecated.

#37 @swissspidy
22 months ago

I think we've already settled the naming/separation discussion.

We may be able to abstract the underlying resource hints code though, so that it can be reused for this new API. This way we can keep two separate names and don't have to deprecate anything.

#38 @mihai2u
22 months ago

"duplicates a near identical API" -> since this a feedback for applying Don't Repeat Yourself principles to the code, yes, it's a nice to have, but as specs evolve, that might introduce more complexities down the road than combining the functions or APIs would.

I can give one relevant example - preloading a module JS requires a different syntax:
<link rel="modulepreload" href="dog.mjs">

And with a separate implementation, this logic can be baked in the preload logic much easier, like when passing a "msj" parameter for the as, to change the output to match the specification:

https://developer.chrome.com/blog/modulepreload/

My suggestion is to keep this as it is for the flexibility it provides for having the code separated.

#39 @mihai2u
22 months ago

Since it was questioned of what use-case is helpful here, I'm sharing the examples which can be added to one of the twenty-X themes to use the new API

Preloading the CSS for a web font which can be loaded in a non-render blocking fashion (early discover and trigger the download, but don’t delay the paint if slow)

<link rel="preload" href="https://fonts.googleapis.com/css2?family=Gulzar&display=swap" as="style">
<link href="https://fonts.googleapis.com/css2?family=Gulzar&display=swap" rel="stylesheet" media="print" onload="this.media='all'">

Preloading responsively the first image in the content
https://web.dev/preload-responsive-images/

<link rel="preload" as="image" href="wolf.jpg" imagesrcset="wolf_400px.jpg 400w, wolf_800px.jpg 800w, wolf_1600px.jpg 1600w" imagesizes="50vw">

When the image lacks srcset/sizes, it defaults to not having those extra attributes

<link rel="preload" as="image" href="important.png">

And when there's a high confidence that the image is above the fold for all devices, there's an extra attribute
fetchpriority="high"
which can be used as such:

<link rel="preload" as="image" href="important.png" fetchpriority="high" ...>

https://youtu.be/fWoI9DXmpdk?t=1205

When an image is not present on particular breakpoints, the media attribute comes into play here:

<link rel="preload" href="mydog" as="image" media="(min-width: 601px)" ...>

Preloading render blocking CSS defined in the head is also still useful, especially in the circumstance of structured data appearing before the stylesheet links, causing the early HTML bytes not to contain the stylesheet, causing a late discovery.

Preloading a script is either

<link rel="preload" as="script" href="https://cdn.ampproject.org/v0.js">

or, for module JS

<link rel="modulepreload" href="dog.mjs">

Preloading fonts require the type attribute and the crossorigin to be present, which should be hard-coded for as="font" types:

<link rel="preload" href="fonts/cicle_fina-webfont.woff2" as="font" type="font/woff2" crossorigin>

I hope I've been exhaustive enough for real world common usages.
Many of these example use-cases can be added to the developer notes with the specific PHP code and the required parameters as it gets released/merged.

#40 in reply to: ↑ 36 @azaozz
22 months ago

  • Keywords has-patch removed

Replying to peterwilsoncc:

I think using the existing code in wp_resource_hints would be better.

Same, and also...

it's not possible to coordinate between preload and hinting to avoid double ups.

Think that fixing this is quite important.

  • What happens when the same resource is "hinted" and "preloaded". Would that "confuse" the browser in some way? If not, still thinking duplication shouldn't happen at least for optimization.
  • Lets assume hints will be outputted first, how do we stop preloading the same stuff?

If anyone feels strongly enough that there is a better name that can encompass both hinting and instructing the browser, the the existing name can be deprecated.

Yes, thinking the two should actually be merged and form the same API as both code and functionality is pretty close.

#41 @azaozz
22 months ago

Also see some inline comments on the PR about naming that can be fixed/improved and resetting/reusing vars inside a loop that should be avoided: https://github.com/WordPress/wordpress-develop/pull/2505/files (in both the new code and in the existing wp_resource_hints()).

#42 follow-up: @mihai2u
22 months ago

From my experience with hints - they get enqueued with a low priority, and encountering a preload bumps up the priority (like from low to medium). This does not have a performance penalty.

Preloads should be output first, as the main use-case for them is if they are present in the initial HTML response, to trigger their download before the HTML is fully downloaded & parsed.

If we configure the defaults for preloads to show up earlier than hints, the presence of a hint later would yield no difference, or any switch in the prioritisation level of the request.

My tests with hints / preloads & preloads with or without the fetchpriority attribute set to high were done back in May, so quite recent to still hold relevancy now.

My opinion is adding extra logic to remove duplication of resources to not show up under a preload and a hint at the same time is not one guardrail we'd like to impose. The web changes quickly enough not to consider forever recommended to do things in X way over another. Having both options to the engineers to use is better.

I myself would consider filling a ticket if I do a resource preload and a resource hint and the hint isn't showing, and could waste some precious minutes trying to figure out why isn't the function behaving like I would expect.

@azaozz if you need specific examples to confirm my own prior conclusions laid above I can create some HTML docs accordingly to back them up. Ask and I'll support.
This is a key feature I've been using often after the HTTP push got its browser support revoked, and it should be part of the WP toolset.

Thanks!

#43 in reply to: ↑ 42 @azaozz
22 months ago

Replying to mihai2u:

From my experience with hints - they get enqueued with a low priority, and encountering a preload bumps up the priority (like from low to medium). This does not have a performance penalty.

Preloads should be output first...

if you need specific examples to confirm my own prior conclusions...

No, no need. It looks like a small optimization to remove duplicates. But if duplicates won't affect the functionality, and even could become useful in the future, lets keep them. My only suggestion would be to add this explanation to the docblock (with a link to here). Even better, add something small to both docblocks. Then if one day this needs to be revisited it will be very easy to understand the choices and reasoning.

In that case the things left here are to see if it makes sense to "merge" wp_resource_hints() and wp_preload_links(), perhaps abstract the common parts and make a lower-level (private use) function called from the higher-level functions. At first look it seems to make sense. But then these are pretty small/simple functions so not sure if it's worth it. What do you think?

If not doing that, thinking the comments about naming and reusing vars in the PR should be addressed, in both wp_resource_hints() and wp_preload_links().

#44 @azaozz
22 months ago

Rereading the older comments, couple more suggestions:

  • Rename wp_preload_links() to wp_preload_resources(). It outputs link tags (same as wp_resource_hints()) but the actual functionality is to preload resources.
  • Open another ticket to update wp_resource_hints() as described in comment 3 and fix/enhance the var names.

Also seems better to keep these two separate.

#45 @adamsilverstein
21 months ago

I'm going to pick up the PR and apply all of the suggested changes from there and the final comments here.

adamsilverstein commented on PR #2505:


21 months ago
#46

@manuelRod 👋🏼 I'm going to pick this up and add the recommended feedback above to help move this ticket forward.

This ticket was mentioned in PR #3041 on WordPress/wordpress-develop by adamsilverstein.


21 months ago
#47

  • Keywords has-patch added

little refactor (printf instead of echo)

Trac ticket: https://core.trac.wordpress.org/ticket/42438

adamsilverstein commented on PR #2505:


21 months ago
#48

@manuelRod - I was unable to push changes here, I don't have permissions. So instead I opened a new PR on my fork - https://github.com/WordPress/wordpress-develop/pull/3041; happy to close that and push changes here instead if you can give me that capability, otherwise will continue work there. If you review the last commit(s) you will see the fixes from the feedback above.

#49 @adamsilverstein
21 months ago

42438.diff is an updated patch based on the original PR. I wasn't able to push to that PR so I opened a new one to run the tests.

  • Rename wp_preload_links() to wp_preload_resources()
  • Rename all variables with clearer naming
  • Remove reset of variable
  • Structural change: break into two loops - upper loop to gather unique resources, lower loop to output HTML for each unique resources

I didn't touch wp_resource_hints, we can update that function and possibly consolidate logic into a helper on a follow up ticket; that shouldn't be a blocker to landing this enhancement.

I also want to address some feedback from @swissspidy -

Are we confident the current API is intuitive enough and can be used to solve typical use cases? Having to use a filter to add a preload link vs. an imperative way (e.g. a wp_add_preload_link() function or so) is arguably not the best developer experience.

I agree it would be good to build an imperative way to add these, however that can work alongside a filter like the one introduced here.

Would you see a wp_add_preload_link function setting some sort of global we could then use as a starting value for the filter?

Already asked 4 years ago: what in WordPress core today would benefit from preload links? We should have some good use cases in core (or bundled themes) prior to adding this. There should be some examples that proof that the proposed API can solve these use cases.

Great question! can we apply this in wp-admin and Gutenberg for example to help prioritize some loading and get a demonstrable performance gain? @mihai2u or @gziolo - any suggestions? It would be great to add some actual core usage of the new capability.

#50 @adamsilverstein
21 months ago

  • Focuses performance added

azaozz commented on PR #3041:


21 months ago
#51

LGTM, good job.

adamsilverstein commented on PR #3041:


21 months ago
#52

Very minor tweaks.

thanks for the refinements @dream-encode - applied your suggested changes.

#53 @adamsilverstein
21 months ago

  • Keywords commit needs-dev-note added
  • Type changed from defect (bug) to enhancement

42438.2.diff includes all of the updates from the PR.

Marking this for commit (as an enhancement) given the approvals on the PR, this is in a good state.

We should follow up with:

  • core usage where we can
  • further cleanup/DRYing out of similar wp_resource_hints function
  • adding a more declarative API as suggested by @swissspidy

#54 @gziolo
21 months ago

Great question! can we apply this in wp-admin and Gutenberg for example to help prioritize some loading and get a demonstrable performance gain? @mihai2u or @gziolo - any suggestions? Adding some actual core usage of the new capability would be great.

I don't know about any obvious usage. However, I've seen this exciting exploration from @tyxla in Gutenberg https://github.com/WordPress/gutenberg/pull/42525, where he plays with Suspense from React to improve the editor loading experience. It might be an excellent opportunity to look at using asset preloading to take the whole concept to another level.

#55 @mihai2u
21 months ago

The most common scenario for preloading is for render-blocking resources which might appear after the first HTML network packet (as they would be discovered a full round-trip later by the download scheduler), as the earliest moments are the ones when there is more network bandwidth available, due to less competition.

For example, jquery-core-js and jquery-migrate-js are two render-blocking resources (always in the admin) that could appear after many inline styles and are prime examples of preloading that can move the network download earlier in the timeline due to the certainty of them being required.

Preload directives should not be used for non-render-blocking elements, as then they would compete with the render-blocking ones slowing down the render.

Last edited 21 months ago by mihai2u (previous) (diff)

#56 @adamsilverstein
21 months ago

The most common scenario for preloading is for render-blocking resources which might appear after the first HTML network packet (as they would be discovered a full round-trip later by the download scheduler), as the earliest moments are the ones when there is more network bandwidth available, due to less competition.

Preload directives should not be used for non-render-blocking elements, as then they would compete with the render-blocking ones slowing down the render.

Makes sense, thanks for clarifying this!

#57 @azaozz
21 months ago

@mihai2u As Adam said, great clarification, thanks :)

Thinking this probably should be added to the docblock for wp_preload_resources(), as preloading can actually slow down rendering if used incorrectly.

Last edited 21 months ago by azaozz (previous) (diff)

manuelRod commented on PR #3041:


21 months ago
#58

@adamsilverstein thanks for taking this over, I've been busy and couldn't get into it. LGTM. Happy to see this feature landing to core finally!

#59 @adamsilverstein
21 months ago

Thinking this probably should be added to the docblock for wp_preload_resources(), as preloading can actually slow down rendering if used incorrectly.

Good suggestion, I'll add some language there

#60 @adamsilverstein
21 months ago

42438.3.diff improves the docblock, explaining that preloads should not be used with non-render-blocking elements.

For the imperative API suggestion, we can probably add a $wp_preload_resources global set with calls to wp_add_preload_link, then use that as the default value for the filter in wp_preload_resources. I'll work on a follow up for that.

#61 @adamsilverstein
21 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53846:

Script loader: enable resource preloading with rel='preload'.

Add a wp_preload_resources filter that developers can use to add resource preloading.

Preloading helps the browser discover and prioritize important resources earlier during the page load. This ensures that they are available earlier and are less likely to block the page's render.

Props nico23, swissspidy, igrigorik, westonruter, azaozz, furi3r, aristath, spacedmonkey, peterwilsoncc, mihai2u, gziolo. 
Fixes #42438.

#64 @SergeyBiryukov
21 months ago

In 53849:

Tests: Rename the test class for wp_preload_resources() tests.

This matches the name of the function being tested.

Follow-up to [53846].

See #42438.

This ticket was mentioned in PR #3089 on WordPress/wordpress-develop by adamsilverstein.


21 months ago
#65

Trac ticket:

#66 follow-up: @adamsilverstein
20 months ago

For a more declarative API, thinking something like 42438-function.diff -

  • Add a function wp_add_preload_resources which accepts an array of resources to preload with the same structure as the wp_preload_resources filter.
  • Since the filter runs later, it can still be used as a final way to direct the preloads, with the function as the recommended approach for adding a preload.
  • Adds testing for the new function, conveniently using the same data provider as the tests for wp_preload_resources.

What do you think @swissspidy / @azaozz ?

#67 in reply to: ↑ 66 ; follow-up: @azaozz
20 months ago

Replying to adamsilverstein:

For a more declarative API, thinking something like...

It would be nice to have wp_add_preload_resources() but I'm not a fan of globals. It is the (very) old way of doing things and has many drawbacks. For example some plugin may start manipulating the global directly. Then if core decided to fix or change something, it will fail.

Also this will fail if wp_add_preload_resources() is used after wp_preload_resources(), and the plugin using it will not even know. Worth mentioning in the docblock at least.

#68 in reply to: ↑ 67 @peterwilsoncc
20 months ago

Replying to azaozz:

Also this will fail if wp_add_preload_resources() is used after wp_preload_resources(), and the plugin using it will not even know. Worth mentioning in the docblock at least.

If the function is added, maybe check if the wp_head action has fired and log a notice if that is the case.

I realise there is wp_preload_resources within wp_preload_resources() but I can see how a plugin developer may wish to use those in combination to add Link HTTP headers.

#69 @furi3r
20 months ago

What are the actual benefits of having a declarative API? I cannot imagine a scenario where the filter is insufficient to achieve what the developer wants.

#70 @adamsilverstein
20 months ago

What are the actual benefits of having a declarative API? I cannot imagine a scenario where the filter is insufficient to achieve what the developer wants.

Good question @furi3r! This was proposed by @swissspidy who may want to weigh in here; in my mind this would be a cleaner way to declare the preload, especially for a script directly after adding it, adding via a filter works but is a bit messier.

It doesn't add a new capability, rather a different way of using the already added capability.

If the function is added, maybe check if the wp_head action has fired and log a notice if that is the case.
Also this will fail if wp_add_preload_resources() is used after wp_preload_resources(), and the plugin using it will not even know. Worth mentioning in the docblock at least.

Ah good point, we could fire a "doing_it_wrong" notice if the function is called after the headers were already output.

I'm not a fan of globals. It is the (very) old way of doing things and has many drawbacks

Admittedly, I was following old patterns! How would you suggest building this? As a class variable? do we have similar code elsewhere in core to mimic?

#71 @peterwilsoncc
18 months ago

@mukesh27 added to props via make/core per comment #32.

Note: See TracTickets for help on using tickets.