Make WordPress Core

Opened 10 months ago

Closed 5 months ago

Last modified 5 months ago

#58632 closed enhancement (maybelater)

Add support for 'async' and 'defer' loading to script that use inline scripts.

Reported by: joemcgill's profile joemcgill Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: needs-patch close
Focuses: performance Cc:

Description (last modified by westonruter)

This is a follow-up to #12009.

While adding support for registering a script with an 'async' or 'defer' loading strategy, the solution that was committed [56033] did not include support for scripts that have an inline script attached in the 'after' position, nor if any script that is a dependent of the script has an inline script attached in the after position. In either scenario, the original script will be printed without the registered loading strategy in order to maintain execution order of the scripts in the dependency chain when an inline script is included.

The purpose of this ticket is to reopen discussion about adding support for this use case. An initial implementation that included support for this use case was included in the initial PR prior to removal in this commit (from squashed PR). Relevant conversation on the initial ticket that led to the decision to remove support for inline scripts starts at this comment.

Change History (11)

#2 @JG Visual
6 months ago

This may not be the best place to note this, but I wanted to add another relevant use case here.

Below I've included the default Google Analytics 4 JavaScript. Ideally you could load the https://www.googletagmanager.com/gtag/js script async by including the strategy. Unfortunately it's not possible since an inline script also needs to be attached.

Given that Google Analytics is used on tons of websites and some developers may prefer to load it without a plugin, allowing for wp_register_script() and wp_enqueue_script() to work as expected here would be helpful.

<!-- Google tag (gtag.js) -->
<script async src="https://www.googletagmanager.com/gtag/js?id=G-00000000"></script>
<script>
  window.dataLayer = window.dataLayer || [];
  function gtag(){dataLayer.push(arguments);}
  gtag('js', new Date());

  gtag('config', 'G-00000000');
</script>

#3 follow-up: @westonruter
5 months ago

  • Keywords close added

@jg-visual Given that the gtag script is designed to be asynchronous, the inline script can actually be placed anywhere in the page, either before or after the gtag script. That's what this window.dataLayer || [] bit of code is about. So you can do this:

<?php
wp_enqueue_script(
        'gtag',
        'https://www.googletagmanager.com/gtag/js?id=G-00000000',
        array(),
        null,
        array( 'strategy' => 'async' ) 
);
wp_add_inline_script(
        'gtag',
        "
                window.dataLayer = window.dataLayer || [];
                function gtag(){dataLayer.push(arguments);}
                gtag('js', new Date());

                gtag('config', 'G-00000000');
        ",
        'before'
);

And this will get rendered successfully as async:

<script id="gtag-js-before">
		window.dataLayer = window.dataLayer || [];
		function gtag(){dataLayer.push(arguments);}
		gtag('js', new Date());

		gtag('config', 'G-00000000');
	
</script>
<script src="https://www.googletagmanager.com/gtag/js?id=G-00000000" id="gtag-js" async data-wp-strategy="async"></script>

On the other hand, if the external script was not designed to be asynchronous:

<?php
wp_enqueue_script(
        'foo',
        'https://www.example.com/foo.js',
        array(),
        null,
        array( 'in_footer' => true )
);
wp_add_inline_script(
        'foo',
        sprintf( 'Foo.init( %s );', wp_json_encode( get_my_foo_args() ) ),
        'after'
);

If you want to use a delayed loading strategy (namely defer) you can also still do this, by wrapping the before inline script in a DOMContentLoaded event handler:

<?php
wp_enqueue_script(
        'foo',
        'https://www.example.com/foo.js',
        array(),
        null,
        array( 'strategy' => 'defer' )
);
wp_add_inline_script(
        'foo',
        sprintf( 'document.addEventListener( "DOMContentLoaded", () => Foo.init( %s ) );', wp_json_encode( get_my_foo_args() ) ),
        'before'
);

So you can see that to port an existing blocking script with an inline after script to use a delayed strategy, you have to: (1) move the inline script from after to before, and (2) wrap the code in the inline script with a DOMContentLoaded event handler. This should work in the majority of cases.

Now, as to whether we should implement automatic support for delaying inline after scripts for delayed scripts, it turns out that usage of after scripts is not particularly high. I queried HTTP Archive for all inline scripts being used in WordPress, and 86%+ are before inline scripts (65.26% via wp_localize_script() and 21.29% via wp_add_inline_script()). Only 9.84% are after inline scripts.

Even though I worked a lot on this feature which got pulled from 6.3, I don't think at the present time there is a real need to implement delayed inline after scripts. I suggest we close this as maybelater.

#4 follow-up: @JG Visual
5 months ago

Thanks a ton for the thoughtful response @westonruter. It's great to know what options are available.

I still have concerns though that the vast majority of developers will end up without the best solution. They'll try to combine the aysnc strategy with an inline script that loads after, resulting in the strategy not applying. Then they either won't realize it's not working, or will be confused why it's not. I only learned what the issue was by diving into the Core PHP. While some developers might do that, many likely won't.

If this won't be supported, should we at least update the documentation for the related functions to note the limitations? We could also output a message in debug.log, but that might be overkill.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

#6 @joemcgill
5 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Based on @westonruter's comment, and the fact that there are already workaround for this, I'm closing as maybelater.

#7 in reply to: ↑ 4 @westonruter
5 months ago

Replying to JG Visual:

I still have concerns though that the vast majority of developers will end up without the best solution. [...] I only learned what the issue was by diving into the Core PHP. While some developers might do that, many likely won't. ¶ If this won't be supported, should we at least update the documentation for the related functions to note the limitations? We could also output a message in debug.log, but that might be overkill.

Where do you think this documentation would best live given your experience as a developer who just encountered this issue? You found it by digging into core. We do have the restriction on inline after scripts documented in a Make/Core post.

#8 in reply to: ↑ 3 @azaozz
5 months ago

Replying to westonruter:

the inline script can actually be placed anywhere in the page, either before or after the gtag script.
...
So you can see that to port an existing blocking script with an inline after script to use a delayed strategy, you have to: (1) move the inline script from after to before, and (2) wrap the code in the inline script with a DOMContentLoaded event handler. This should work in the majority of cases.

That looks great! Imho simpler, faster, and more elegant than handling this from PHP. Very nice :)

Where do you think this documentation would best live

One suggestion would be to have the tl;dr in a docblock, perhaps for wp_add_inline_script(), and link to the make/core post, or maybe even do another post specifically on how to run scripts after (pretty much copy/paste the above comment).

#9 @JG Visual
5 months ago

I agree that adding something into the docs at https://developer.wordpress.org/reference/functions/wp_add_inline_script/ would be helpful. The content within Make/Core is thorough and clearly explains the situation, but it's part of a really long post that many developers likely won't read. I'm also not sure we can assume they'll find that post if they're starting point is the wp_add_inline_script() docs.

Thanks for the back and forth.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

#11 @joemcgill
5 months ago

In the future, to reconsider this change, I think we should conduct another analysis in the to see how often async/defer scripts are being downgraded to blocking scripts due to this issue, using the data-wp-strategy attribute we add to mark intended strategy for debugging purposes.

Note: See TracTickets for help on using tickets.