[wp-trac] [WordPress Trac] #12009: Add support for HTML 5 "async" and "defer" attributes

[WordPress Trac]

#12009: Add support for HTML 5 "async" and "defer" attributes
-------------------------------------------------+-------------------------
 Reporter:  Otto42                               |       Owner:  10upsimon
     Type:  enhancement                          |      Status:  assigned
 Priority:  high                                 |   Milestone:  6.3
Component:  Script Loader                        |     Version:  4.6
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests 2nd-        |     Focuses:
  opinion                                        |  performance
-------------------------------------------------+-------------------------
Changes (by azaozz):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests 2nd-
               opinion


Comment:

 Replying to [comment:113 joemcgill]:
 > > However imho a bigger concern is the proposed JS that handles "scripts
 after". Cloning a node and injecting it in the DOM as `text/javascript` is
 pretty much as bad as running that node's content with `eval()`.
 >
 > I’m in total agreement with your concerns here. When testing this
 approach with Content-Security-Policy, @westonruter
 [https://github.com/WordPress/wordpress-
 develop/pull/4391#issuecomment-1536869109 discovered] that it had a
 security vulnerability related to...

 Yea, this looks a bit better but is still "not-a-good-idea". The problem
 is with the content of a `type="text/template"` node being run with JS
 `exec()`. As mentioned many times in many places, this is generally a bad
 idea. So unless that script is outputted (printed) as
 `type="text/javascript"` (or the `type` attribute omitted) I don't think
 it would be a good addition to core.

 However the above is not the only reason to remove that code. There are
 other reasons:
 1. This is not used in core. Adding untested and unconfirmed API methods
 to core is a bit red flag. The "unconfirmed" part refers to usability,
 i.e. if that method is not used in core, would it at least be useful to at
 least 10% of the plugins that use Script Loader? As far as I see this is a
 "NO".

 > This goes back to why inline scripts were introduced in the first place.
 There needs to be a way to pass dynamic data from PHP back to static JS
 scripts that can execute either before or after a registered script.

 2. This is not exactly true :)
 What you're saying is true for "before" scripts, but not for "after"
 scripts. There is actually very little need for "after" scripts, so this
 feature of Script Loader has always been on the border of being quite
 useless. I.e. nearly all cases where an "after" script is used can be
 handled by using a "before" script. There are only few rare exceptions
 (which frankly do not warrant the existence of the convenience method for
 adding of "after" scripts, but that's a different question of a "bad API
 decision" that was made some time ago) .

 > {{{#!php
 > wp_register_script(
 >       'foo',
 >       plugin_dir_url( __FILE__ ) . 'foo.js',
 >       array(),
 >       '0.1',
 >       array( 'strategy' => 'defer' )
 > );
 > wp_add_inline_script(
 >       'foo',
 >       sprintf( 'Foo.init( %s )', wp_json_encode( getFooData() ) ),
 >       'after'
 > );
 > }}}

 This example is "the incorrect way to output data for scripts in
 WordPress" :) The "correct" way (using a "before" script) can be seen in
 many places in Script Loader:

 - https://core.trac.wordpress.org/browser/tags/6.2.2/src/wp-includes
 /script-loader.php#L885
 - https://core.trac.wordpress.org/browser/tags/6.2.2/src/wp-includes
 /script-loader.php#L920
 - https://core.trac.wordpress.org/browser/tags/6.2.2/src/wp-includes
 /script-loader.php#L981
 - https://core.trac.wordpress.org/browser/tags/6.2.2/src/wp-includes
 /script-loader.php#L1001
 - https://core.trac.wordpress.org/browser/tags/6.2.2/src/wp-includes
 /script-loader.php#L1021

 and many many more. (Note that `$scripts->localize()` adds a "before"
 script.)

 > Additionally, we are attempting to create safe upgrade paths for scripts
 that are currently blocking to be converted to a defer or async loading
 strategy. We've designed the API changes to ensure that inline scripts
 added to a handle continue to be executed in the intended order when the
 loading strategy changes.  This removes an important backward
 compatibility concern...

 Yea, this is a lofty goal but unfortunately there is no 100% backwards
 compatibility. Since the loading order of "async" scripts is undetermined,
 they are not backwards compatible when used as dependency.

 Yea, I know this was discussed above and the decision was to "leave it to
 the plugins to decide what to do about it". Not having support for "script
 after" is inline with that decision.

 So the TL;DR: for "script after" support for "async" and "defer" scripts:
 1. Doesn't use a good, safe methods in JS.
 2. Currently very rarely used (for "blocking" scripts). Most uses are not
 warranted.
 3. Not used in core (and seems it will never be used).

 In these terms I'm starting to get curious why this strong push to add
 that to core? :)
 If there is really such a big desire, a better option would be to make a
 canonical plugin that implements it, imho. That way all developers that
 really want to use such code can have access to a standard way of handling
 it (can even copy the plugin's code, etc.). It will be really interesting
 to see usage stats for such plugin before considering such additions to
 core.

 Re-adding the "2nd opinion" label as I think there are many unanswered
 questions here.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/12009#comment:115>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform