Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 6 months ago

#58870 closed enhancement (fixed)

Delay loading comment-reply script with async loading strategy

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.3
Component: Comments Keywords: add-to-field-guide
Focuses: javascript, performance Cc:

Description (last modified by westonruter)

Currently the comment-reply script is loaded in the footer. This is great because it doesn't block rendering, however it can still block other scripts from executing and thus it delays DOMContentLoaded. This means that if a theme has a script with logic running at DOMContentLoaded to add interactivity to the nav menu, this will get delayed for the sake of comment-reply which is not even needed unless the user scrolls down the page to the comments. In the worst case scenario, a user may try to tap on the nav menu too early and nothing may happen due to comment-reply causing an unnecessary delay.

Adding either defer or async to the comment-reply script will cause the browser to treat it as low priority from a loading perspective. However, in the case of defer it will still block other defer scripts and the DOMContentLoaded event. With async there is the possibility that the execution may be delayed until after the DOM loads if the script is not in the cache yet. On the other hand, if the async script is cached, it may execute right away which would be no different than the current situation of it being a blocking script in the footer: so no better or worse. But for the case where the comment-reply script is not cached yet, async would surely be better.

Additionally, according to Barry Pollard: "Current browsers may execute all defer scripts altogether in one single task (Chrome does). There is talk within the Chrome team as to whether these should run in their own tasks. Using async would prevent adding another script to this defer group."

Change History (10)

This ticket was mentioned in PR #4876 on WordPress/wordpress-develop by @westonruter.


9 months ago
#1

  • Keywords has-patch added

#2 @westonruter
9 months ago

  • Owner set to westonruter
  • Status changed from new to accepted

#3 @westonruter
9 months ago

  • Description modified (diff)

#4 follow-up: @sergiomdgomes
9 months ago

Hey folks! 👋

Thank you for the extra insight regarding defer scripts and the fact that they can all get executed as a single task. I wasn't aware of that, that's good info! In any case it sounds like an implementation detail that we probably shouldn't be optimising specifically around (particularly if it's limited to a single browser engine, and even more so if there's talk about potentially changing that); but we can definitely take it into account assuming the workarounds don't cause problems elsewhere.

---

So let's look at our options here. The first axis of choice is whether the script should live in the <head> or at the end of the <body> (the footer, in WP terms).

To me, it seems that if this is an unimportant script that we don't mind executing late, it should live in the footer. This means it won't start fetching as early; even if it would do so at a lower priority, it could still start and compete for some bandwidth (in Chrome, low priority items will use less bandwidth but can still start before all higher priority items are finished, and thus delay them somewhat in constrained bandwidth situations).

As such, it seems there's no obvious reason to include it in the <head>, but please correct me if I'm missing something.

Which leads us to the next orthogonal choice: blocking, defer, or async. Both blocking and defer will delay DOMContentLoaded until the script is fetched and executed. async will only delay DOMContentLoaded if the script is already in cache; i.e., the script may execute after DOMContentLoaded if it's not fetched in time. async will not preserve source order, but that doesn't sound like a concern here.

So, if our goal is to:

  • ensure first render isn't blocked
  • ensure DOMContentLoaded is blocked in as few situations as possible
  • (best-effort) prevent long tasks in browsers that currently stitch defers together

then the best solution seems an async script in the footer.

If the script needs to live in the <head> for whatever reason, however, then I'd argue that the top priority is to ensure that first render isn't delayed, in which case our only reliable option is defer.

Does this reasoning make sense, @westonruter? Is there something I missed?

Thanks again for starting this discussion! 👍

Last edited 9 months ago by sergiomdgomes (previous) (diff)

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

Replying to sergiomdgomes:

then the best solution seems an async script in the footer.

If the script needs to live in the <head> for whatever reason, however, then I'd argue that the top priority is to ensure that first render isn't delayed, in which case our only reliable option is defer.

Does this reasoning make sense, @westonruter? Is there something I missed?

Yes, I think that summarizes the situation well!

#6 @westonruter
9 months ago

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

In 56323:

Script Loader: Delay loading comment-reply script with async loading strategy.

Props westonruter, flixos90, joemcgill, sergiomdgomes.
See #12009.
Fixes #58870.

#8 @spacedmonkey
6 months ago

  • Keywords add-to-field-guide added

#9 follow-up: @webcommsat
6 months ago

  • Keywords has-patch removed

@westonruter do you have some draft text for this to appear in the Field Guide? There is no dev note label, can we please confirm that there will not be a dev note to link to from the Field Guide to accompany any text on this change?

The Field Guide is published on Tuesday.

This is the [link to the Documentation tracker]https://github.com/WordPress/Documentation-Issue-Tracker/issues/1159 for this change. Thank you for your work on this ticket and change, and for assistance with information for the Field Guide.

I have removed 'needs patch' as this ticket has been committed.

#10 in reply to: ↑ 9 @westonruter
6 months ago

Replying to webcommsat:

@westonruter do you have some draft text for this to appear in the Field Guide? There is no dev note label, can we please confirm that there will not be a dev note to link to from the Field Guide to accompany any text on this change?

Dev note: https://make.wordpress.org/core/2023/10/17/script-loading-changes-in-wordpress-6-4/

Last edited 6 months ago by westonruter (previous) (diff)
Note: See TracTickets for help on using tickets.