[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
-------------------------------------------------+-------------------------

Comment (by westonruter):

 Thanks for the quick reply, @azaozz.

 Replying to [comment:125 azaozz]:
 > Currently the inline "after scripts" may run (be placed in the HTML
 source) long after the "main" script. Example: when using script
 concatenation (with load-scripts.php which is always used in production)
 they are outputted after the concatenated script that contains all "main"
 scripts. [...] However the bigger problem here is that this method does
 not match (doesn't have parity with) the way "after" scripts currently
 work (mostly) in production, see above.

 I’m confused because previously
 [https://core.trac.wordpress.org/ticket/12009?replyto=125#comment:86 you
 said] that "support for concatenation would not be a priority and probably
 won't be necessary. This would simplify it a bit.” You’ve also proposed in
 #57548 that we eliminate script concatenation entirely since it is
 obsolete. Nevertheless, `load-scripts.php` isn’t used in production on the
 frontend; it’s only in the admin.

 In any case, Script Loader always skips concatenation for any handle to
 which an inline script is attached. This can be confirmed in
 [https://github.com/WordPress/wordpress-
 develop/blob/bea4307daa21a3f97d159898b0ac676332e0e7de/tests/phpunit/tests/dependencies/scripts.php#L575-L601
 the existing unit tests].

 > Seems that to match this behaviour the "after" scripts for delayed main
 scripts would have to be executed on the DOMContentLoaded event, or
 another appropriate event, that ensures that all delayed scripts have
 finished executing. This could be done from PHP by wrapping each "after"
 script in a lambda JS function that would execute at the appropriate time.
 ¶  Looking further, using this method would guarantee that the "after"
 scripts will be executed after the delayed main scripts in all cases,
 regardless of when and where the inline scripts appear in the HTML.
 However as the Script Loader API is a JavaScript API (not PHP), adding
 that lambda function may be considered "too much hand-holding", as this is
 a very well known, very basic pattern in JS. ¶ As mentioned above, support
 for "after" scripts can either be revised to achieve parity with current
 concatenated scripts, or completely removed and left for themes/plugins to
 handle it in JS. If removed there should be enough documentation
 explaining why, and how to add inline scripts that would execute after the
 main script has finished executing. Seems the most appropriate way would
 be to run any "after" scripts on the DOMContentLoaded event.

 We actually considered relying on the `DOMContentLoaded` event to run the
 after inline scripts, but the problem with this is that existing inline
 after scripts may be attempting to define something in the global scope
 without explicitly setting it on the `window`. For example:

 {{{#!js
 wp_add_inline_script( 'foo', 'var fooHasLoaded = true;' );
 }}}

 Such an after inline script would break if it were automatically wrapped
 in an event handler callback. So instead our approach of dynamically
 inserting the `script[type="text/plain"]` after scripts is fully backwards
 compatible since anything defined in the function will go into the global
 scope.

 > Even more, these inline scripts can be outputted directly by the plugins
 on different PHP "actions" or with different hook priority. There is
 actually no need for plugins to add any "before" or "after" inline scripts
 in Script Loader at all as long as the JS is written properly. (I wouldn't
 call this "best practice", but it is possible.)

 Right, this would not be a best practice as it disconnects the inline
 scripts from the scripts they relate to, making dequeuing them difficult
 or impossible.

 > > 2. The implementation method in the current PR for delaying inline
 scripts is unsafe
 > Wouldn't call it "unsafe", just "unneeded" and perhaps a bit "weird" :)

 It doesn’t seem particularly weird when compared with how external scripts
 often get dynamically inserted into the page (i.e. lazy-loading JS). For
 example, the [https://github.com/WordPress/wordpress-
 develop/blob/ba9a2f8b/src/js/_enqueues/lib/emoji-loader.js#L152-L158
 emoji-loader does this]:

 {{{#!js
 function addScript( src ) {
         var script = document.createElement( 'script' );

         script.src = src;
         script.defer = script.type = 'text/javascript';
         document.getElementsByTagName( 'head' )[0].appendChild( script );
 }
 }}}

 This can also be seen
 [https://github.com/mediaelement/mediaelement/blob/0dec971/src/js/utils/dom.js#L12-L27
 in MediaElement.js] and
 [https://github.com/WordPress/gutenberg/blob/1d9c4fa/packages/block-
 editor/src/components/iframe/index.js#L89-L102 in Gutenberg]. The only
 difference is that we’re doing this for inline scripts instead of external
 ones.

 > The question here is: do the browsers handle <script> tags exactly in
 the same way and with the same security restrictions like <script type
 ="something-else"> tags? Think that wasn't the case some time ago, not
 sure now. You're right, the use of nonces seems to remove many security
 concerns.

 Good to hear!

 > > 3. Supporting async and defer could conflict with future dynamic
 module imports
 > Thanks for looking at this! Yes, seems that dynamic module loading will
 be a browser controlled "pure JS" thing, completely "outside" of Script
 Loader.

 Agreed. Nevertheless, I think Script Loader is still useful to enqueue
 module scripts, even though the dependencies array may be empty. Even so,
 the dependencies could end up being useful still as they could perhaps be
 added to a `script[type=importmap]`.

 > > ...where data from the server is being supplied to an inline script in
 an after position
 > Yea, this is generally "doing_it_wrong" as long as Script Loader is
 concerned. See examples from all previous inline scripts that supply data
 (like the old translated strings). While it is possible to do I'd consider
 it "bad practice" for any JS that is handled by Script Loader.

 The problem with the examples in Script Loader that are using before
 inline scripts is that they require the use of a global variable. There
 are a [https://github.com/search?q=repo%3AWordPress%2Fwordpress-
 develop%20add_inline_script&type=code couple dozen examples] of after
 inline scripts being used in core which allow data to be passed directly
 to the script without involving a global variable. This is beneficial for
 a few reasons, including: (1) reduces memory usage, (2) prevents pollution
 of global scope, (3) allows existing libraries to be re-used without
 having to work in a shim to make them look for global variables for
 configuration. So to me it seems like quite a good practice. Some examples
 of passing data to a script via an after inline script include
 [https://github.com/WordPress/wordpress-develop/blob/3e2121c/src/wp-
 includes/block-editor.php#L642-L649 block-editor.php],
 [https://github.com/WordPress/wordpress-develop/blob/3e2121c/src/wp-admin
 /site-editor.php#L110-L114 site-editor.php], and
 [https://github.com/WordPress/wordpress-develop/blob/3e2121c/src/wp-
 includes/general-template.php#L3909 general-template.php].

 > Can make the core scripts "non-portable" for no good reason.

 How do you mean?

 > Don't think that would ever be possible because of the edge cases. It
 would open a huuuge can of worms :) ¶ I'm actually thinking the current PR
 may need to include a way to prevent (and add "doing_it_wrong") cases
 where a plugin or theme attempts to re-register any core script with async
 or defer. Unfortunately seems this functionality could be used only for
 newly added core scripts as there's no way to ensure 100% back-compat for
 the loading and execution order.  ¶ Do you think a method to detect when a
 plugin or a theme "tweaks" a core script to add defer or async is worth
 adding? Can probably see if that becomes common and then add it?

 Actually, being able to transition applicable core scripts to use a
 delayed strategy is specifically what we want to explore once this gets
 merged (e.g. `comment-reply`). We’ve specifically developed our
 implementation in a way that retains compatibility between core delayed
 scripts and theme/plugin blocking scripts. In particular, if a core script
 adopts a delayed strategy but a theme/plugin is enqueueing a blocking
 script that depends on the core-delayed script, then our implementation
 forces the core script to revert to being blocking. So we have implemented
 a back-compat method to maintain the loading and execution order,
 including for before/after inline scripts.

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