WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 2 months ago

#12009 reopened enhancement

Add support for HTML 5 "async" and "defer" attributes

Reported by: Otto42 Owned by: azaozz
Milestone: Future Release Priority: normal
Severity: normal Version: 4.6
Component: Script Loader Keywords:
Focuses: Cc:

Description (last modified by scribu)

HTML5 supports async and defer attributes on script tags: http://www.w3.org/TR/html5/semantics.html#attr-script-async

Basic usage of these:

  • "async" scripts get executed asyncronously, as soon as they're loaded. This lets them run without slowing down the page parsing (normally, all page processing stops while the javascript code is executing).
  • "defer" scripts get deferred from running until page processing is complete. Sorta like jQuery(document).ready() does, except without pre-definitions. Faster, in other words, since it's built into the browser.

Correct usage would dictate that "libraries" like jQuery and such would get the async attribute, while bits of code that use the current DOM would get deferred. The defer bit is basically optional though, since most all code that exists uses something like jQuery(document).ready() already, when it's necessary, and so there's not a lot of benefit there.

The just released Firefox 3.6 supports the async attributes, so you can do testing with these immediately. I've noticed a speedup on the wp-admin side of things by using it, but I have not measured this and cannot be sure I'm not imagining it. Still, it does seem like it makes the page appear faster.

Attachments (4)

defer and async.patch (2.5 KB) - added by scep 6 years ago.
async_defer_scripts.patch (6.4 KB) - added by wpnook 23 months ago.
Adds defer/async attribute option for wp_enqueue_scripts
async_defer_scripts_new.patch (3.4 KB) - added by wpnook 23 months ago.
Removes code unrelated to this ticket
async_defer_scripts_new.2.patch (3.4 KB) - added by wpnook 23 months ago.
Removes leftover print_r

Download all attachments as: .zip

Change History (64)

#1 @scribu
8 years ago

  • Milestone changed from Unassigned to 3.0

We could add a new parameter to the script loader: load_type, with one of the following values:

  • (normal loading)
  • 'defer'
  • 'async'

#2 @nacin
8 years ago

  • Milestone changed from 3.0 to Future Release

This seems like a good idea to eventually do, but no patch or traction yet. Future release for now.

#3 @ptahdunbar
7 years ago

  • Cc trac@… added
  • Keywords needs-patch added

#4 @azaozz
7 years ago

This sounds good but to implement it and actually use it we would need at least the majority of the browsers in use to support it properly (as per the HTML5 specs). This seems to be "very far in the future" for now.

#5 follow-up: @scribu
6 years ago

  • Description modified (diff)

Alternatively, we could just add a filter to wp_print_scripts(), allowing users to add attributes when they need to.

As always, users find weird workarounds when the API doesn't oblige. For example, using the clean_url filter:

http://wordpress.stackexchange.com/a/38335/205

#6 @toscho
6 years ago

  • Cc info@… added

#7 @scep
6 years ago

  • Cc scep added
  • Keywords has-patch added; needs-patch removed

I was also looking for this feature, saw that it did not exist so i took a shot at it writing it in, I attached a patch for what I did.

#8 @scep
6 years ago

  • Keywords needs-testing added

#9 in reply to: ↑ 5 @azaozz
6 years ago

Replying to scribu:

As always, users find weird workarounds when the API doesn't oblige...

Agreed. However I'm still feeling a bit "uneasy" about adding support for a feature that is not supported by more than half of the browsers currently in use: http://en.wikipedia.org/wiki/Usage_share_of_web_browsers.

#10 follow-up: @toscho
6 years ago

These attributes are backwards compatible. I don’t think supporting them would do any harm.

But I don’t like more arguments for wp_register_script(). We have already four, that’s more than enough. How about a very simple extra string in WP_Scripts::do_item()?

$src = esc_url( apply_filters( 'script_loader_src', $src, $handle ) );
		
$extra = apply_filters( 'script_extra', '', $src, $handle );

if ( $this->do_concat )
	$this->print_html .= "<script type='text/javascript' src='$src' $extra></script>\n";
else
	echo "<script type='text/javascript' src='$src' $extra></script>\n";

#11 @azaozz
6 years ago

Yes, another filter would do the trick but has many drawbacks. If we decide to use these attributes in core, there will be no good way to add them: it will interfere with what plugins are adding (or removing) in all cases.

#12 in reply to: ↑ 10 ; follow-up: @scep
6 years ago

Replying to toscho:

These attributes are backwards compatible. I don’t think supporting them would do any harm.

But I don’t like more arguments for wp_register_script(). We have already four, that’s more than enough. How about a very simple extra string in WP_Scripts::do_item()?

I agree that since they are backwards compatible they should be supported.

I also agree that my patch adds a lot of extra arguments for the register and enqueue functions. I like you filter idea but it seems like it would lead to having a lot of code in the return function to identify which scripts should have extra info attached to them.

#13 in reply to: ↑ 12 @scribu
6 years ago

Replying to scep:

I also agree that my patch adds a lot of extra arguments for the register and enqueue functions. I like you filter idea but it seems like it would lead to having a lot of code in the return function to identify which scripts should have extra info attached to them.

Every script has a unique ID which it can be identified by (the first parameter to wp_enqueue_script()).

#14 @wycks
6 years ago

Here is the current Compatibility (Aug 2012):

Firefox: yes
Chrome: yes
Safari: yes
Andriod: yes
Blackberry: yes
IE version 10: yes


IE 6, 7, 8, 9: no

Opera: no (and no future plans)


http://caniuse.com/#search=async

ps. For reference the much used google analytic code uses async by setting var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;

#15 @georgestephanis
6 years ago

Leaving a comment to remind myself to come back and write a patch for this later. This is a great example of graceful degradation that probably should be supported.

#16 @Otto42
6 years ago

I like the patch, but instead of adding new params to the function calls, lets change the $in_footer parameter to be allowed as an array of attributes.

Basically, instead of $in_footer = false, have $attr = false.

For backward compatibility, you could do if $attr === true, then $attr = array('in_footer'=>true).

Then, you can have $attr = an array of the various settings, including potential future ones, allowing you to define whether it's in the footer or not, defer or not, async or not, etc.

Last edited 6 years ago by Otto42 (previous) (diff)

#17 @scribu
6 years ago

Or, we could just throw our hands in the air: add a 'script_loader_tag' and let plugins add whatever attributes they see fit: #13592

#18 @ryanve
6 years ago

The ideal way to handle script/style attributes is via #22249 because it is future-proof for any new attributes that are invented.

The 'script_loader_tag' should be added for consistency with the already existing 'style_loader_tag' to allow for customizations that go beyond attributes, such as wrapping in IE conditionals or adding inline scripts. (#13592 and #22245)

Version 0, edited 6 years ago by ryanve (next)

#19 @kevinlangleyjr
5 years ago

  • Cc kevinlangleyjr added

#20 @lkraav
5 years ago

  • Cc leho@… added

#21 @nacin
5 years ago

The 'script_loader_tag' should be added

I agree.

#22 @pothi
5 years ago

  • Cc pothi added

#23 @nacin
4 years ago

  • Component changed from JavaScript to Script Loader

#24 @webaware
4 years ago

A further complication: CloudFlare Rocketscript. This is a service that compresses and bundles scripts from your website into a single download. It does this by replacing your script elements with ones that look like this:

<script type='text/rocketscript' data-rocketsrc='http://example.com/path/to/src.js?ver=1.0'></script>

After the page has loaded, Rocketscript collects those elements all together and loads / executes the scripts. Which means scripts that don't like being loaded asynchronously break of course.

To disable Rocketscript for specific scripts, you can add an attribute data-cfasync="false" as per this support note:

https://support.cloudflare.com/hc/en-us/articles/200169436

However: it only works if the attribute comes before the src attribute.

So I like Toscho's solution, but with $extra added into the script element before the src attribute, not after it.

FYI: this is how I solved the problem for a plugin user, without such extra attribute support; nasty but effective. I'd love to offer a better solution :)

https://gist.github.com/webaware/8949605

#25 @SergeyBiryukov
4 years ago

#27672 was marked as a duplicate.

#26 @grapplerulrich
3 years ago

Could this be closed as the filter script_loader_tag has been added? #13592

#27 @wonderboymusic
3 years ago

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

#28 @ocean90
3 years ago

  • Milestone Future Release deleted
  • Resolution changed from fixed to worksforme

#29 @wpnook
23 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Version set to trunk

Is there any interest in reviving this topic? I was going to create a ticket, but found this ticket, so I'll submit my patch here. Browser support seemed to be a big part of the debate previously and shouldn't be a concern now.

The lack of options for using the async and/or defer attributes with script elements is a bit of a pain in WordPress, which has resulted in some users coming up with dirty/hackish ways of doing so, such as using the clean_url filter to add the attribute(s).

The script_loader_tag solves this to a degree, but it would be better to specify this option at the time of loading the script (the filter has other uses as well).

This patch adds two new arguments for {{{wp_enqueue_script}} and outputs the results in the printed <script> tag, and both default to false.

Using both tags simultaneously may be beneficial for legacy browser support, and some third-party documentation (Google Maps JS API for example) recommends using the attributes together.

@wpnook
23 months ago

Adds defer/async attribute option for wp_enqueue_scripts

@wpnook
23 months ago

Removes code unrelated to this ticket

#30 @SergeyBiryukov
23 months ago

  • Milestone set to Awaiting Review

@wpnook
23 months ago

Removes leftover print_r

#31 @azaozz
23 months ago

  • Keywords has-patch needs-testing removed

FTR async and defer do pretty much the same thing: begin to download the script in parallel immediately. The difference is that each async script executes after it has finished downloading. It’s likely that async scripts are not executed in the order in which they occur in the page. The defer scripts are guaranteed to be executed in the order they occur in the page (https://webkit.org/blog/1395/running-scripts-in-webkit/).

Then logically:

  • async and defer make little sense when scripts are concatenated.
  • async scripts cannot be used as dependencies.
  • defer scripts can be used as dependencies only if all dependent scripts also have the defer attribute.
  • Neither async nor defer scripts can have any inline scripts.

In that terms the above patch is a good start but is missing a lot. Looks like this will be quite complex to implement in WP_Scripts. Perhaps it can be made a bit simpler by supporting these attributes only for third party scripts, etc.

#32 @mor10
22 months ago

FWIW I support adding the option of async / defer to a regular enqueue call at the individual script level. This should not be an automated process but a developer decision on a script-by-script basis to ensure proper hierarchy etc. Requiring a filter seems excessive considering all we're talking about is two possible properties without variables or values. As HTTP/2 rolls out, this is becoming a more relevant issue because script concatenation is an anti-pattern in the new multiplexed reality.

#33 @seancjones
15 months ago

I think this is an important issue to resolve. As of now, Google Page Speed Insights failing for a lot of WordPress installs can almost exclusively be blamed on render-blocking JavaScript. Fixing that increases speed by wide margins.

Plugin developers should have a way of deferring page loads. There are clear pitfalls to avoid - like people's websites breaking.

I'm going to play devil's advocate and argue why a filter might be better.

A filter allows a user with a code sample to defer scripts, or remove scripts if they appear to break their current setup. Plugin developers can send an array of script handles to defer via a plugin, rather than rewrite their code with a new parameter.

If a user wanted to disable all deferring:

add_filter ( 'wp_defer_scripts', function($wp_defer) {
  return array();
}, 900, 2 );

add_filter ( 'wp_async_scripts', function($wp_async) {
  return array();
}, 900, 2 );

For someone to enable all hooks:

add_filter ( 'wp_async_scripts', function($wp_async) {
    global $wp_scripts;

    return $wp_scripts->queue;
} );

This would also make it far easier for plugin developers to patch their plugins.

#34 @georgestephanis
13 months ago

Personally I'd prefer not to have the defer / async as arguments in the register/enqueue script functions, but require a wp_script_add_data() call afterwards to flag it on -- the same way we do for rtl support.

Quick tangential gist I threw together for testing of something related, if anyone wants to play with deferring now:

https://gist.github.com/georgestephanis/2a84bc55ad23f4dec2cf2464109add59

#35 @LindsayBSC
12 months ago

Personally I would appreciate the allowance for setting script tag attributes when the script itself is registered but I can also see the need for allowing filtering of it. For that reason I put together this little bit of code to append to (and rewrite) the section of do_item in class.wp-scripts.php that does the actual generation of the tag.

My thinking is that the <script> is the actual tag, and everything else including the src and type are attributes. We should be treating attributes similarly and thus I felt it best to generate all of the attributes at the same time. This could also allow plugins to filter the order in which they are output.

It is somewhat of a combination of the methods suggested in #22249 and @Otto42 's suggestions

I didn't have access to my dev environment to create a diff patch file, but here is the code concepts in a multi-site gist with the appropriate file names listed alone with the general line number where the code would go.

https://gist.github.com/LinzardMac/dc951d7fabf3fbeb1f10794c3130fecc

Last edited 12 months ago by LindsayBSC (previous) (diff)

#36 @mor10
3 months ago

async / defer is now considered best practice yet we still can't do it in WordPress. Let's get this sorted.

This ticket was mentioned in Slack in #core-js by mor10. View the logs.


3 months ago

#38 @gziolo
3 months ago

Can we also include the support for nomodule and type.module when dealing with other script attributes? I think there are even more. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script. I think that support for ES6 modules (https://hacks.mozilla.org/2015/08/es6-in-depth-modules/) would open some great ways to serve smaller bundles for developers writing their code with the latest language features. It still limited to a small number of browsers, but it has this nomodule fallback so it's more than okey to have it available as a progressive enhancement.

#39 follow-up: @azaozz
3 months ago

async / defer is now considered best practice...

Correct me if I'm wrong but both async and defer don't make much difference for scripts at the end of the HTML document (in the footer).

In addition they don't make sense when scripts are concatenated, i.e. in production.

In addition async cannot be used if the script has dependencies or is used as dependency for another script.

So, what are the user cases for these attributes with the current script-loader?

As far as I see the only thing we can do is add defer to the script tag that loads the concatenated scripts in the footer. We probably can add defer when outputting single script tags for scripts that are enqueued for the footer. Not sure if either of these will make any significant difference :)

Also thinking we should have another look at script-loader. Modern JavaScript is about modules and building (pre-concatenating). The current script-loader's dependency model and JIT concatenation may not be the best for it.

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

#40 in reply to: ↑ 39 ; follow-up: @mor10
3 months ago

Replying to azaozz:

async / defer is now considered best practice...

Correct me if I'm wrong but both async and defer don't make much difference for scripts at the end of the HTML document (in the footer).

The modern best practice is to load JavaScripts in the head section of the document and use async / defer to prevent render blocking. This allows the browser to pull in and cache the JavaScript files while waiting for the DOM to load. See https://developers.google.com/speed/docs/insights/BlockingJS

In addition they don't make sense when scripts are concatenated, i.e. in production.

Concatenating JavaScript is an old patch to get around single-stream transfer under HTTP/1.1. With HTTP/2 multiplexing it is an anti-pattern that slows down performance. Componentizing, loading, and async / defer scripts in head + theoretically leveraging server-push dramatically improves performance.

In addition async cannot be used if the script has dependencies or is used as dependency for another script.

Not entirely accurate. If you defer a group of scripts, they will load and run in the document order, so dependencies still work. Putting that aside, this is the reason why async and defer need to be optional attributes that can be added at enqueue level.

So, what are the user cases for these attributes with the current script-loader?

One immediate use case: navigation.js in _s should be loaded in head and deferred. Same with skip-link-focus-fix.js. Doing so removes render blocking and improves performance. As I said, async / defer is now best-practice.

As far as I see the only thing we can do is add defer to the script tag that loads the concatenated scripts in the footer. We probably can add defer when outputting single script tags for scripts that are enqueued for the footer. Not sure if either of these will make any significant difference :)

IMO async / defer should have their own optional parameters in wp_enqueue_script() and wp_register_script() to give developers the ability to choose whether scripts should be loaded by default behavior, asynced, or deferred. Not doing so stands in the way of modern best practices.

Also thinking we should have another look at script-loader. Modern JavaScript is about modules and building (pre-concatenating). The current script-loader's dependency model and JIT concatenation may not be the best for it.

Probably, though I think that's a separate conversation. async / defer should be handled immediately.

#41 @mor10
3 months ago

There is significant overlap between this ticket and #22249.

In my view, async / defer is a separate conversation from general script attributes as these specific attributes are booleans and play unique roles in how the browser renders scripts. Separating the two conversations will allow us to move forward on solutions for each of them in a more meaningful way. Right now there is a lot of crosstalk between the tickets and the two different issues are discussed as if they are one and the same, which in my opinion they are not.

#42 follow-up: @westonruter
3 months ago

  • Milestone changed from Awaiting Review to Future Release

In terms of async_defer_scripts_new.2.patch, and as @Otto42 commented, I think that we should be getting away from adding new positional parameters to wp_enqueue_script(). Already they are hard to remember. Adding two additional ones will just make it harder to use. I suggest we rework wp_register_script() to allow passing the $deps, $ver, and $in_footer as part of an $args array, which would allow for them all to be omitted (since they are optional) as well as to easily allow for new parameters including async, defer, and module. For example:

<?php
wp_register_script( 'foo', plugins_url( 'foo.js', __FILE__ ), array(
    // Look ma, no $deps or $ver!
    'in_footer' => true,
    'async' => true,
    // ... 'defer', 'module', etc.
) );

Same pattern can be applied to wp_enqueue_script().

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

#43 in reply to: ↑ 40 ; follow-up: @azaozz
3 months ago

Replying to mor10:

Concatenating JavaScript is an old patch to get around single-stream transfer under HTTP/1.1. With HTTP/2 multiplexing...

All of this sounds pretty good, in theory. I wish we could do that right now. The problem is that less than 25% of the websites currently support HTTP/2: https://w3techs.com/technologies/details/ce-http2/all/all.

In addition async cannot be used if the script has dependencies or is used as dependency for another script.

Not entirely accurate.

The async attribute cannot be used in script-loader because the script doesn't follow execution order. If you are adding a stand-alone script that has no dependencies and other scripts don't depend on it, there is no point adding it to script loader at all.

Right now plugins and themes can load all scripts in the head and add defer if they really want by using the 'script_loader_tag' filter. This will make 75% of sites slower. Whether we should make this easier is another question. I'd rather we don't.

#44 in reply to: ↑ 43 ; follow-up: @mor10
3 months ago

Replying to azaozz:

All of this sounds pretty good, in theory. I wish we could do that right now. The problem is that less than 25% of the websites currently support HTTP/2: https://w3techs.com/technologies/details/ce-http2/all/all.

This is where I say we (WordPress, 30% of the web if you believe the stats) have a role to play in moving the web forward. We also have a role to play in making sure we don't hold the web back. While HTTP/2 is still emerging, it is rapidly doing so, and the uptick will increase once Chrome and Firefox release their "this site is not secure" flags and GDPR come into effect. The way WordPress handles scripts today is not the way the web of right now handles scripts, nor the way modern browsers and servers want us to handle scripts. We need to push forward on this to establish best practices and grant those who want to build modern web applications the capability to do so without having to build around how WordPress handles JavaScript.

#45 in reply to: ↑ 42 @mor10
3 months ago

Replying to westonruter:

For example:

<?php
wp_register_script( 'foo', plugins_url( 'foo.js', __FILE__ ), array(
    // Look ma, no $deps or $ver!
    'in_footer' => true,
    'async' => true,
    // ... 'defer', 'module', etc.
) );

Same pattern can be applied to wp_enqueue_script().

This would be a solid step forward IMO. It makes it easier to understand what's happening, and allows deeper level of control. The one thing I'd add is we need a way of preventing someone from accidentally setting both async and defer to true at the same time as they conflict.

#46 in reply to: ↑ 44 ; follow-up: @azaozz
3 months ago

Replying to mor10:

While HTTP/2 is still emerging, it is rapidly doing so, and the uptick will increase once Chrome and Firefox release their "this site is not secure" flags and GDPR come into effect. The way WordPress handles scripts today is not the way the web of right now handles scripts, nor the way modern browsers and servers want us to handle scripts.

I agree, in principle. But then why do this a teaspoon at a time? Lets implement it "properly" for all current scripts.

Currently there are few tickets that touch on some parts of updating/refactoring script-loader. IMHO best would be to combine them and really refactor it. Implement HTTP/2 + defer, output all in the head, retire load-scripts.php, etc. etc. Ideally this should be done before Gutenberg lands in core.

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

#47 in reply to: ↑ 46 ; follow-up: @westonruter
3 months ago

Replying to azaozz:

Currently there are few tickets that touch on some parts of updating/refactoring script-loader. IMHO best would be to combine them and really refactor it. Implement HTTP/2 + defer, output all in the head, retire load-scripts.php, etc. etc. Ideally this should be done before Gutenberg lands in core.

Retiring load-scripts.php I think can also/rather be accomplished via service workers: #36995.

The async attribute cannot be used in script-loader because the script doesn't follow execution order. If you are adding a stand-alone script that has no dependencies and other scripts don't depend on it, there is no point adding it to script loader at all.

It would make sense to me that if you register a script with an async flag and a non-empty deps that this should result in a _doing_it_wrong().

It makes sense to me to use the WP dependency system even if there are no dependencies because it provides a standard programatic interface for registering and outputting scripts. Core or a plugin could register an async script and then it or other plugins could just simply enqueue it and be done with it, as is standard in WP. A plugin could check to see if a script is already registered and prevent registering and enqueueing their own if it already exists. Also no worrying about adding an action to print a manually-crafted script tag. No (less) worrying about duplicated scripts being output.

Currently there are few tickets that touch on some parts of updating/refactoring script-loader. IMHO best would be to combine them and really refactor it. Implement HTTP/2 + defer, output all in the head, retire load-scripts.php, etc. etc. Ideally this should be done before Gutenberg lands in core.

Just to be clear, I'm talking about WP script dependencies generally not load-scripts.php specifically. I think there is good value in adding async and defer support to the existing WP_Scripts system so that we can take advantage of them now. Since you noted that less than 25% of the Web is on HTTP/2, I don't think we should let it be the blocker for us to encourage best practices now for script loading for themes and plugins via async and defer. HTTP/2 definitely won't be available before Gutenberg.

#48 follow-up: @mor10
3 months ago

For clarity: We should not blanket defer all scripts. A decision as to what approach is best (default, async, or defer) needs to be done on a script-by-script basis to optimize performance. The behavior of the two options is significantly different and can be taken advantage of in different circumstances.

Separately, just to add more numbers to the convo, the Can I Use stats on support are relevant: https://caniuse.com/#search=http2

#49 in reply to: ↑ 48 @azaozz
3 months ago

Replying to mor10:

For clarity: We should not blanket defer all scripts. A decision as to what approach is best (default, async, or defer) needs to be done on a script-by-script basis to optimize performance.

I don't think so. Mixing the "old" and "new" way of adding scripts will result in a big mess. A typical user case:

  • I enqueue myscript.js to load the old fashioned way in the footer.
  • It depends on jquery-ui-dialog which depends on most of jQuery UI.
  • However another plugin decides it wants to load jquery-ui-position with defer. Deferred scripts will be executed after the footer scripts which breaks the execution order for myscript.js and possibly some of the other dependencies.

To prevent this from happening we will have to actively ban use of defer for all default scripts in core (not even sure we can do that easily).

In that case, what would be the benefits of a plugin adding its own script with defer?

  • All core dependencies will still have to be loaded in the footer.
  • HTTP2 won't work on 75% of the sites, it will end up slowing them down.

Where is the optimization here? Isn't that rather saying: "We added support for defer but it will not work the way it should".

I'm going to repeat myself: if we want to make this better we should "go for broke", refactor script-loader and fully implement loading of scripts as it should be in 2018. No point of doing little things that don't make much of a difference :)

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

#50 in reply to: ↑ 47 ; follow-up: @azaozz
3 months ago

Replying to westonruter:

Retiring load-scripts.php I think can also/rather be accomplished via service workers: #36995.

Right. This can be done in couple of different ways. The point is that it makes sense as part of refactoring how we load scripts in general.

It would make sense to me that if you register a script with an async flag and a non-empty deps that this should result in a _doing_it_wrong().

I agree we can add async when registering scripts that don't have any dependencies and can never be used as a dependency. This makes most sense for default core scripts that plugins can use.

Just to be clear, I'm talking about WP script dependencies generally not load-scripts.php specifically. I think there is good value in adding async and defer support to the existing WP_Scripts system so that we can take advantage of them now.

Yes but load-scripts.php is an integral part of how we load scripts. Using async and defer will have to pull a script out of there (prevent concatenation) and break the execution order. We can't do that for any of the default scripts, see the previous comment.

Since you noted that less than 25% of the Web is on HTTP/2, I don't think we should let it be the blocker for us to encourage best practices now for script loading for themes and plugins via async and defer. HTTP/2 definitely won't be available before Gutenberg.

Yep, I realise that. Refactoring how we load scripts will slow down 3/4 of the sites currently. However looking at the speed HTTP/2 is being implemented, that number will be quite lower in six months, and possibly be less than 50% by the end of the year.

I also agree with @mor10 that implementing it will give another push to faster switching to HTTP/2 everywhere.

This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.


3 months ago

#52 @mor10
3 months ago

Relevant: Introducing HTTP/2 Server Push with NGINX 1.13.9 https://www.nginx.com/blog/nginx-1-13-9-http2-server-push/

#53 @markcallen
2 months ago

I just wanted to add a comment about filters on output. It is perhaps the subject of a new discussion or ticket.

If any output is a html tag, then I believe it should be an array of attribute => value pairs that is filtered.

This gives full control to developers to add, view, edit or remove attributes based on their specific needs. It is more resilient to successive filters being used and more importantly it works for any attributes - current or future.

String manipulation which is relied on too much for a lot of filters is just ugly and messy and often gets implemented assuming that no previous filter has been called.

In the case of this ticket, this approach would allow developers to add async/defer attributes based on their specific implementations. It wouldn't need other updates to core and is future proof to any other cool attributes that are created in the future.

Whether developers use that to enhance or degrade performance is a matter for the developer, in my opinion. I agree with @azaozz, better to refactor script-loader for 2018 - with my point above included on eventual output :)

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

#54 in reply to: ↑ 50 @westonruter
2 months ago

Replying to azaozz:

Yes but load-scripts.php is an integral part of how we load scripts. Using async and defer will have to pull a script out of there (prevent concatenation) and break the execution order. We can't do that for any of the default scripts, see the previous comment.

Something else to clarify here is that I think @mor10 is primarily eyeing use of async and defer on the frontend where wp-admin/load-scripts.php is not used (or at least I've never seen it used), and in which theme/plugin scripts are not included for concatenation anyway (since they are not in_default_dir).

But in the admin this is also where I think service workers should be used to cache assets instead (as I noted above), and here too this would allow for the scripts to cease being concatenated and so there would be no need to pull out async/defer scripts.

I see value in pushing forward with async/defer support in core for the immediate term, as these are standard performance best practices today. They'd be used primarily by themes and plugins: I can see some core scripts like comment-reply.js get async, but we'd just need to work into the script-loader logic to strip async from any script that is a dependency for another script.

Then as HTTP/2 and service workers become more well supported we can abandon load-scripts.php entirely and potentially make async/defer conditional based on whether HTTP/2 is available, for example.

#55 @westonruter
2 months ago

Also, in regards to concatenation and load-scripts.php... I don't believe there is any reason why an async/defer script would have to be pulled out from being concatenated. Browsers should still execute such scripts just fine if they are loaded synchronously, as these attributes were introduced and could be used even before all browsers supported them. So we could keep serving the scripts inside of load-scripts.php.

#56 @ocean90
2 months ago

In terms of implementation, please let no change the signature of the current functions. Neither adding another argument nor changing the third argument to support an array. I think that's a bad practice even if we have done this in the past.

As @georgestephanis has already suggested in comment:34, using wp_script_add_data() should be the preferred way to add support for this.

But instead of using boolean values we can use a specific key like script-execution which can be defer or async:

wp_script_add_data( 'my-script', 'script-execution', 'defer' );
wp_script_add_data( 'my-script', 'script-execution', 'async' );

#57 follow-up: @westonruter
2 months ago

@ocean90 here's a(nother) plugin implementation (and polyfill) for what you describe:

<?php
/**
 * Add async/defer attributes to enqueued scripts that have the specified script_execution flag.
 *
 * @link https://core.trac.wordpress.org/ticket/12009
 * @param string $tag    The script tag.
 * @param string $handle The script handle.
 * @return string
 */
function wp12009_filter_script_loader_tag( $tag, $handle ) {
        $script_execution = wp_scripts()->get_data( $handle, 'script_execution' );
        if ( ! $script_execution ) {
                return $tag;
        }
        if ( 'async' !== $script_execution && 'defer' !== $script_execution ) {
                return $tag; // _doing_it_wrong()?
        }

        // Abort adding async/defer for scripts that have this script as a dependency. _doing_it_wrong()?
        foreach ( wp_scripts()->registered as $script ) {
                if ( in_array( $handle, $script->deps, true ) ) {
                        return $tag;
                }
        }

        // Add the attribute if it hasn't already been added.
        if ( ! preg_match( ":\s$script_execution(=|>|\s):", $tag ) ) {
                $tag = preg_replace( ':(?=></script>):', " $script_execution", $tag, 1 );
        }

        return $tag;
}
add_filter( 'script_loader_tag', 'wp12009_filter_script_loader_tag', 10, 2 );

Then to make comment-reply.js async all that is needed is as you showed and @georgestephanis suggested:

<?php
add_action( 'wp_enqueue_scripts', function() {
        wp_script_add_data( 'comment-reply', 'script_execution', 'async' );
} );

I like not using a boolean attribute, but the script_execution name doesn't feel quite right yet.

#58 in reply to: ↑ 57 @westonruter
2 months ago

Replying to westonruter:

Abort adding async/defer for scripts that have this script as a dependency. _doing_it_wrong()?

On second thought, I don't think it should do this error auto-detection. Async scripts can still have dependencies, but they are not synchronous dependencies. In other words, there should be a dependency in place so that the async script gets printed as well as its (async) dependency; it's just that in the case of async scripts, the order doesn't matter. If we are going to do any such detection, we perhaps should flag when a non-async script gets printed which itself has an async script dependency. Or rather we should just not worry about warning at all since the developer will find a JS error when they test anyway.

#59 @mihai2u
2 months ago

I concur with weston's train of thought that doing this type of validation seems to go beyond the scope of this ticket. If you're setting up async, you should know what you're doing and what you're applying it to.

There's a different path this can go in as well... If you're deferring a script which is usually setup as a dependency (let's think jquery), it would be nice to defer all of the scripts that have it as a dependency, as that will keep everything in functioning order even when one more plug-in is added which has a jQuery dependency, instead of it breaking. But that's a whole other story altogether, we can find improvements to these in time. It's already been 8 years!!! since this ticket was opened, we should get a simple implementation in.

#60 @nikolay.yordanov
2 months ago

When this is implemented, core inline scripts should be refactored, so that they do not depend on other scripts before DOMContentLoaded. For example, this won't work if jquery.js is deferred: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/script-loader.php#L1286

Note: See TracTickets for help on using tickets.