#34292 closed feature request (fixed)
Support for DNS Prefetching & Prerender
Reported by: | bhubbard | Owned by: | voldemortensen |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Script Loader | Keywords: | has-patch has-unit-tests needs-docs |
Focuses: | performance | Cc: |
Description
I would like to see WordPress improve support for DNS Prefetching.
I have written several plugins and themes for clients, and sometimes I want to prefetch various resources for each. At the same time other plugins or themes may try prefetching the same url as well, or I have multiple plugins that want to prefetch the same domain. Having the ability for developers to register and enqueue prefetch dns urls would be great. I'm thinking this might work similar to scripts and stylesheets:
wp_register_dns_prefetch(); wp_enqueue_dns_prefetch();
wp_register_prefetch(); wp_enqueue_prefetch();
wp_register_prerender(); wp_enqueue_prerender();
There are several plugins out there already that offer input boxes to create DNS Prefetch, and Jetpack now has a DNS Prefetch for the urls/resources it loads on a install.
Useful Links:
https://www.chromium.org/developers/design-documents/dns-prefetching
https://jack.ofspades.com/prefetching-preloading-and-prerendering-content-with-html/
Attachments (11)
Change History (73)
#2
@
9 years ago
+1 to have it in core. A standard for adding this to WordPress would help minimize the duplication- which is kind of disturbing to even think of. :)
#3
@
9 years ago
I think having it in core would be very useful, as it can prevent developers from loading duplicates as @moonomo mentioned.
Example , say I have a theme and plugin, and they both want to load and prefetch twitter bootstrap or fontawesome from bootstrapcdn.com. If the both manually print to wp_head, then it would show up twice, having a filter would prevent this. This would also make it easier for developers to say write a function that disables prefetch for a specific page, etc, regardless of what was set to prefetch from another theme or plugin developer, assuming they all switch to using this new filter.
#4
@
9 years ago
I'm pretty sure the usage for DNS Prefetching will increase by a lot from different developers, specially those who will use WordPress REST-API.
#6
@
9 years ago
I just added 34292.diff as a foundation for further discussions. It'd be easy to extend that for prerendering as well, though browser support isn't that great there yet.
#9
follow-up:
↓ 10
@
9 years ago
This patch still applies cleanly and looks good to me (just need to update the @since). I'd like to see this considered for 4.6.
#10
in reply to:
↑ 9
;
follow-up:
↓ 11
@
9 years ago
Replying to voldemortensen:
This patch still applies cleanly and looks good to me (just need to update the @since). I'd like to see this considered for 4.6.
That's awesome! Happy to work more on this.
Some notes:
- The patch is for DNS prefetching only, without prerender. If we do this, we should probably add both
- Is a filter enough for this, or do we need
add_*
/remove_*
helper functions? (I don't think we need them) - Is
array_unique()
enough? IIRC, Jetpack has stricter rules for the added URLs
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
9 years ago
Replying to swissspidy:
That's awesome! Happy to work more on this.
Some notes:
- The patch is for DNS prefetching only, without prerender. If we do this, we should probably add both
- Is a filter enough for this, or do we need
add_*
/remove_*
helper functions? (I don't think we need them)- Is
array_unique()
enough? IIRC, Jetpack has stricter rules for the added URLs
34292.2.diff includes prerendering and unit tests for that. I think a filter is good enough here. If there is demand we can add helper functions later without back compat worries no? And I think array_unique
is good enough here. We don't care about array keys. And I don't think its terrible if someone inadvertently puts a non-url there. Either a bad dns lookup or a 404/couldn't load type page will be cached in the background.
#12
in reply to:
↑ 11
@
9 years ago
Replying to voldemortensen:
Replying to swissspidy:
That's awesome! Happy to work more on this.
Some notes:
- The patch is for DNS prefetching only, without prerender. If we do this, we should probably add both
- Is a filter enough for this, or do we need
add_*
/remove_*
helper functions? (I don't think we need them)- Is
array_unique()
enough? IIRC, Jetpack has stricter rules for the added URLs34292.2.diff includes prerendering and unit tests for that. I think a filter is good enough here. If there is demand we can add helper functions later without back compat worries no? And I think
array_unique
is good enough here. We don't care about array keys. And I don't think its terrible if someone inadvertently puts a non-url there. Either a bad dns lookup or a 404/couldn't load type page will be cached in the background.
Looks pretty neat- thanks for this!
#13
follow-up:
↓ 15
@
9 years ago
I don't think that this belongs to core in it's current state because it does nothing.
There are currently two plugins which are providing this feature: https://wordpress.org/plugins/dns-prefetch/ and https://wordpress.org/plugins/automatic-dns-prefetch/. Both together have only 300+ installs.
I suggest to take a look at the second one because "it parses through WordPress scripts and styles and create DNS prefetch statements for external resources".
#15
in reply to:
↑ 13
@
9 years ago
Replying to ocean90:
I don't think that this belongs to core in it's current state because it does nothing.
There are currently two plugins which are providing this feature: https://wordpress.org/plugins/dns-prefetch/ and https://wordpress.org/plugins/automatic-dns-prefetch/. Both together have only 300+ installs.
I suggest to take a look at the second one because "it parses through WordPress scripts and styles and create DNS prefetch statements for external resources".
Agreed. Core needs to benefit from that, otherwise it doesn't make sense to include it.
Not to forget Jetpack, which has some more installs and uses a similar method.
I'd suggest doing two things:
- Automatically add hosts from enqueued media to the prefetch array
- Add the most important screens to the prerender array. For example, after logging in you are in the dashboard. From there, we could prerender the post-new screen.
This ticket was mentioned in Slack in #core by voldemortensen. View the logs.
9 years ago
#17
@
9 years ago
If/when this is added to core, it should include a preconnect for s.w.org
, to fix #33210.
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
9 years ago
#19
@
9 years ago
34292.4.diff limits the prerendered posts on is_home to 1 by default (filterable). It also prerenders previous and next posts on is_single. It also prefetches all scripts and styles that are enqueued.
Admin prerendering still needs to be improved. It currently prerenders post-new.php on every page in the admin. The paths should be selected carefully. And prerendering maybe shouldn't be the way to go, but instead prefetching things like editor scripts and the like.
#20
@
9 years ago
I also just realized I didn't update the unit tests to reflect the changes I made, so those will need to be redone.
This ticket was mentioned in Slack in #core by voldemortensen. View the logs.
9 years ago
#22
@
9 years ago
Couple of thoughts around adding support for the pre* specs:
- Pre-fetching or -rendering posts on the front-end increases the potential for a site to be brought down if it gets linked by a high-profile site or twitter user. If this path is followed, I’m slightly keener on prefetching than rendering but this is gut rather than science.
- For resources enqueued prior to output, adding the pre* instructions as HTTP headers would start to get WordPress ready for HTTP/2 with server push. Actual push support can be a separate ticket.
- It would be handy to include the preload spec, while the spec itself is in draft support is starting to appear in browser nightlies. Preload is a direction to the browser, prefetch is a suggestion.
This ticket was mentioned in Slack in #core by chriscct7. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by voldemortensen. View the logs.
8 years ago
#26
@
8 years ago
What about preconnect?
It's useful for themes having Google Fonts.
https://css-tricks.com/prefetching-preloading-prebrowsing/
says "Much like the DNS prefetch method, preconnect will resolve the DNS but it will also make the TCP handshake, and optional TLS negotiation."
so having an option of doing something like this will be handy:
<link href='https://fonts.gstatic.com' rel='preconnect' crossorigin>
#27
follow-up:
↓ 28
@
8 years ago
@superpoincare the patch I've got (which I hope to get up today), will allow all pre* specs. Technically it will allow anything you want, so when new specs get added its as simple as filtering it in.
#28
in reply to:
↑ 27
;
follow-up:
↓ 29
@
8 years ago
Replying to voldemortensen:
@superpoincare the patch I've got (which I hope to get up today), will allow all pre* specs. Technically it will allow anything you want, so when new specs get added its as simple as filtering it in.
Great. After you are done, will someone open a ticket for adding preconnect for Google Fonts for Wordpress themes?
#29
in reply to:
↑ 28
;
follow-up:
↓ 30
@
8 years ago
Replying to superpoincare:
Replying to voldemortensen:
@superpoincare the patch I've got (which I hope to get up today), will allow all pre* specs. Technically it will allow anything you want, so when new specs get added its as simple as filtering it in.
Great. After you are done, will someone open a ticket for adding preconnect for Google Fonts for Wordpress themes?
The short answer is no. Once this patch is in place a theme that is using Google Fonts can add preconnect for itself. It would be a bad idea to add a preconnect to Google on every WordPress site because not every WordPress site is going to use Google Fonts.
#30
in reply to:
↑ 29
@
8 years ago
Thanks. I wasn't clear. What I meant was not enabled by default but for Wordpress themes such as Twenty Twelve ... Twenty Sixteen etc. So a separate ticket for those themes asking their authors to include your created functions in their individual theme functions.php files.
#31
@
8 years ago
This patch removes all prerendering by default because after thinking on it for a few weeks, I think it could be bad for bandwidth usage. However, this does add a preconnect for s.w.org and and adds some prefetching for scripts commonly used in wp-admin. Tests have also been updated.
#32
follow-ups:
↓ 35
↓ 36
@
8 years ago
34292.6.diff is an alternative patch with a more simplified approach.
- Changes the naming of the function to
wp_resource_hints()
, aswp_browser_hints()
to me sounds like a browsehappy.com browser nag. Resource hints is more accurate. - Moves the admin parts into that function as it's only a few lines of code and not much added complexity.
- Simplifies the logic to get rid of
unique_browser_hints()
andadd_prefetch_attr()
- Improves unit tests to test the admin parts as well. Moves the various
$preexpected
assertions to a separate test. - Adds one single
wp_resource_hints
filter.
To do:
- Add test for scripts and styles enqueuing.
- Add assertion for the
parse_url()
part fordns-prefetch
- Decide whether the filter makes sense inside the
foreach
loop or if it should be outside.
#33
@
8 years ago
I have added a ticket for preconnect Google Fonts for themes twenty eleven, twelve, thirteen, fourteen, fifteen, sixteen:
This ticket was mentioned in Slack in #core by ocean90. View the logs.
8 years ago
#35
in reply to:
↑ 32
@
8 years ago
Replying to swissspidy:
34292.6.diff is an alternative patch with a more simplified approach.
- Changes the naming of the function to
wp_resource_hints()
, aswp_browser_hints()
to me sounds like a browsehappy.com browser nag. Resource hints is more accurate.- Moves the admin parts into that function as it's only a few lines of code and not much added complexity.
- Simplifies the logic to get rid of
unique_browser_hints()
andadd_prefetch_attr()
- Improves unit tests to test the admin parts as well. Moves the various
$preexpected
assertions to a separate test.- Adds one single
wp_resource_hints
filter.
+1 to all of this
To do:
- Add test for scripts and styles enqueuing.
- Add assertion for the
parse_url()
part fordns-prefetch
- Decide whether the filter makes sense inside the
foreach
loop or if it should be outside.
I think the filter should be kept inside the foreach. If I understand correctly, that would make it easier to filter based on preconnect/prefetch/prerender/etc, instead of having to deal with multiple arrays and making sure to return the whole array of arrays instead of just your modified part.
IMO, once the tests are written for those pieces this is good for commit.
#36
in reply to:
↑ 32
@
8 years ago
Replying to swissspidy:
- Decide whether the filter makes sense inside the
foreach
loop or if it should be outside.
Additionally, keeping the filter inside the foreach will allow us to use array_unique
, because it doesn't do multidimensional arrays.
#37
@
8 years ago
34292.7.diff added the missing tests. It also fixed some issues with parse_url. PHP_URL_HOST wasn't quite enough.
#38
@
8 years ago
Shouldn't css/edit.css
and the other scripts include a version argument for cache busting?
#39
@
8 years ago
34292.8.diff adds cache buster to scripts/styles.
#40
@
8 years ago
@voldemortensen Thanks, but seems like you didn't update the tests.
I think for v1 we should go in without the admin stuff because currently they only make sense for /src installs...
#42
@
8 years ago
34292.9.diff removes admin prefetching for a v1.
#46
@
8 years ago
- Keywords needs-docs added
wp_resource_hints_scripts_styles()
doesn't seem like a suitable function name. This function looks through registered scripts and styles and returns the unique hosts - that's it. The function name should reflect that, to make it descriptive for re-use in other contexts. The fact it's only currently used in core for wp_resource_hints()
shouldn't be a factor.
The description also seems off for what this function does, and it's missing a @return
tag.
The function also seems to look through all registered scripts and styles, even if they may not be enqueued (used)? Is that correct?
#47
follow-up:
↓ 48
@
8 years ago
I am not sure if you want to DNS prefetch for all scripts and styles enqueued.
This is because if the script is in the head, the browser automatically connects to the server. So adding a prefetch might make the browser connect twice.
#48
in reply to:
↑ 47
;
follow-up:
↓ 51
@
8 years ago
Replying to superpoincare:
I am not sure if you want to DNS prefetch for all scripts and styles enqueued.
This is because if the script is in the head, the browser automatically connects to the server. So adding a prefetch might make the browser connect twice.
From the specification:
For example, the application may provide the following resource hints to the user agent:
- The list of origins from which resources will be fetched.
- The next likely user action or navigation target, and the resources that will be required, based on current page or application state, individual user history, or aggregated session data.
So yeah, it makes more sense to add resource hints for enqueued assets instead of registered ones. Especially since the browser only does the DNS prefetching for these, so the actual retrieval will be faster.
Developers can then build upon this to add resource hints for the next likely user targets (e.g. the most recent post when on the homepage etc.). We didn't include this on purpose.
#49
follow-up:
↓ 50
@
8 years ago
@GaryJ Thanks for pointing out! That function really can be optimized.
In 34292.10.diff:
- Rename
wp_resource_hints_scripts_styles()
towp_dependencies_unique_hosts()
(other suggestions welcome) - Only returns unique hosts for enqueued dependencies. Otherwise you get unneeded entries for fonts.googleapis.com and stuff like that
- Fixes docs
- Removes duplicated code by looping over
$wp_scripts
and$wp_styles
as they both extend theWP_Dependencies
class
Worth noting: The unit tests still pass with this patch applied. They shouldn't, i.e. we need a new test.
#50
in reply to:
↑ 49
@
8 years ago
Replying to swissspidy:
- Rename
wp_resource_hints_scripts_styles()
towp_dependencies_unique_hosts()
(other suggestions welcome)
I'd probably have gone for wp_get_dependencies_unique_hosts()
to indicate that something is returned. Not a big issue, but fits in with general understanding of "get" in function names.
#51
in reply to:
↑ 48
@
8 years ago
Replying to swissspidy:
My comment was less about whether a script or style is registered or if it's enqueued. Things such as preconnect are useful for resources fetched by the page but later. It isn't useful to preconnect to say a jquery loaded from jquery.com in the head. If the script is enqueued in the footer, then it's useful to preconnect.
#52
follow-ups:
↓ 54
↓ 57
@
8 years ago
@superpoincare Ah, gotcha! From my understanding, it won't hurt the performance. From the spec:
The user agent should attempt to initiate a preconnect and perform the full connection handshake (DNS+TCP for HTTP, and DNS+TCP+TLS for HTTPS origins) whenever possible, but is allowed to elect to perform a partial handshake (DNS only for HTTP, and DNS or DNS+TCP for HTTPS origins), or skip it entirely, due to resource constraints or other reasons.
Even when enqueued in the head of the document, the resource hints will be delivered to the browser much earlier because their higher up in the source and therefore likely in another HTTP packet. Already being able to perform all necessary handshakes only benefits the actual retrieval of the assets later on.
In addition to that, keep in mind that wp_resource_hints()
doesn't add preconnect
entries for scripts/styles. It adds dns-prefetch
entries, which means the browser only performs a DNS lookup for these resources. So even if preconnect
for scripts/styles enqueued in the header would be bad, dns-prefetch
would cause less harm.
If you're worried about that, you can always use the wp_resource_hints
filter to remove these:
<?php function trac34292_example_resource_hints( $hints, $relation_type ) { if ( 'dns-prefetch' === $relation_type ) { return array_diff( $hints, wp_get_dependencies_unique_hosts() ); // Note the function name might be different } return $hints; } add_filter( 'wp_resource_hints', 'trac34292_example_resource_hints', 10, 2 );
Even on larger sites there are usually few external scripts and styles in the header, so I don't think there's much of an issue here though.
#53
@
8 years ago
Including a default preconnect
entry for s.w.org
seems unjustified. This means every single visitor (with a supporting browser) to every single WordPress site in the world will be performing a preconnect to s.w.org
, purely to save a few ms in case an emoji is used that's not supported by the browser.
Does this really justify a preconnect to s.w.org
on every site?
#54
in reply to:
↑ 52
@
8 years ago
Replying to swissspidy:
Thanks. Will try.
My point was that even dns-prefetch isn't always good. If I have Google Fonts enabled in the head, the browser will fire the request ASAP. So dns-prefetch to fonts.googleapis.com isn't that useful.
What is in fact useful is preconnecting to fonts.gstatic.com.
In fact I have opened a ticket for themes: #37171
I think these things are best left to theme developers who can use Wordpress functions added thanks to this thread in their themes.
Also agree with @johnbillion. Seems like s.w.org preconnect is only needed if the page has emojis. So probably left to the Wordpress persons (could be you yourself!) who are in charge of emoji code.
Thanks again.
#55
@
8 years ago
I agree with @johnbillion and @superpoincare:
Please, don't do pre-something to s.w.org. I don't use anything on there and that would be an extra connection to all my users that could potentially make the whole loading slower.
#57
in reply to:
↑ 52
@
8 years ago
Replying to swissspidy:
That code gives a 500 error.
I also have to add that preconnect is also costly to the server. How much is Wordpress' share? 25%? If all preconnect to s.w.org, it is costly to the servers.
Ilya Grigorik says https://www.igvita.com/2015/08/17/eliminating-roundtrips-with-preconnect/,
Preconnect is an important tool in your optimization toolbox. As above examples illustrate, it can eliminate many costly roundtrips from your request path — in some cases reducing the request latency by hundreds and even thousands of milliseconds. That said, use it wisely: each open socket incurs costs both on the client and server, and you want to avoid opening sockets that might go unused.
#58
@
8 years ago
@superpoincare please note @swissspidy's comment about the function name being in flux, this may be causing your 500 error.
A few notes:
- I expect @barry has opinions about preconnecting to the CDN, I'm not overly concerned about preconnecting from the client side but Barry may be less keen for the sake of the CDN. We could add the preconnect headers using JavaScript in the detection script if needs be and preconnect while twemoji runs.
- There is some validity to avoiding pre* for tags in the HTML header, if the HTML header before script output is > 14k (typical TCP packet size) a site has bigger problems. They could be removed later but that would raise backcompat concerns at the time.
A couple of misses:
- s.w.org is always https
- there are two emoji CDNs, SVG and PNG, they happen to be on the same host by default
- the filter on the emoji CDNs is missing
Re 34292.10.diff:
- works, only shows for queued dependencies
- is it more performant to run
in_array
on each iteration of the loop or wrap the return statement inarray_unique
. Does it even matter?
I'll work up a patch covering the misses based off 34292.10.diff.
#59
@
8 years ago
34292.11.diff refreshes 34292.10.diff:
- covers misses mentioned above
- adds unit test to ensure registered but not enqueued dependencies are not hinted.
#61
@
8 years ago
There is a bit too much going on here. Each issue should have its own ticket. Can someone please do so?
#62
@
8 years ago
- Resolution set to fixed
- Status changed from reopened to closed
Created forks per request above, closing this off as fixed and creating forks for the followup.
#37385: Resource hinting: only dns-prefetch enqueued resources.
#37386: Resource hinting: filter emoji CDNs
#37387: Resource hinting: Do not preconnect to emoji CDN.
#37388: Resource hinting: only dns-prefetch resources in HTML footer
This could be achieved using a single filter, let's say
dns_prefetch
. I'm not sure it's worth to add this to core though, as it's not a big deal to manually print<link rel="dns-prefetch" ...
onwp_head
.