-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Defer enqueued scripts with an experimental toggle #102
Comments
Edit: Added some more differences between module scripts and classic scripts. |
I don't think this is something that can be safely done by default. I think it would have to be done on a case-by-case basis. In other words, it should use an allowlist rather than a denylist for when to apply. For example, So yeah, I think we should:
See the trac ticket for adding support for |
Excellent research here @sgomes . I tend to agree with @westonruter 's comments. Another thing to consider is that by setting This has huge impacts for 3rd-party libraries that generally want to be loaded as early as possible, such as GAM (Google Ad Manager) as well as revenue generating scripts like Pubmatic (Header Bidding) I think it would be extremely useful to extend the current params for
Just my 2 cents! |
Thank you both for your comments, @westonruter and @dainemawer! Let me respond to your comments inline before a broader rethink at the end.
I just want to emphasise that the idea is never to do this by default, but rather to have a toggle that enables a user to try these optimisations with the awareness that they may break their site. This is meant as a very imperfect workaround for a rigid API, that will only work in some cases.
Yes, that's something I failed to mention, thank you for bringing that up! However, I want to note that the limitation exists for a reason: because blocking parsing to modify the document is generally a bad practice, from a performance point of view (and from other points of view as well). That doesn't change the fact that it's being used and would break, of course, but I'd argue that it's reasonable to expect folks that want to turn on the proposed toggle to have to refactor their code away from
That is definitely a safer option and something to be considered. However, it's very limited. First of all, I think we should definitely never use As for Furthermore, there can be a big difference between removing 1KB of blocking scripts and removing the last 1KB of blocking scripts. Some of these performance benefits only realise themselves once there are absolutely no blocking scripts and the browser doesn't even have to initialise the JS engine before rendering — although in all fairness that is based on old Chrome behaviour, and may no longer apply to modern browser engines.
Thank you for pointing me to that! I do happen to be aware of that ticket, but I think that it's a flawed approach in general. To explain, we've established that we can only really defer a dependency without introducing breakage if all dependants are deferred as well. This means we have one of three options for when that's not the case:
All of these options seem less than ideal as part of an official API.
I don't think that's entirely correct. Any script that isn't marked as As for
I haven't looked at GAM specifically, but it's generally sufficient to load most libraries when the document is ready. If you're making use of something for which this is not sufficient (the best example I can think of are JS-driven A/B testing libraries, which by their nature need to rewrite the document before it renders), then you're probably better off not enabling the proposed toggle. Note that these are still bad practices from a performance perspective, even if they happen to be popular in some circles! So in my mind, it makes sense if that makes them incompatible with an optional toggle that tries to improve performance. Thank you again for your comments, @westonruter and @dainemawer! I did fail to mention in the issue text that this is not meant as a long-term solution, which really doesn't help with the discussion 😄 What I see as a long-term solution is not this, nor the ongoing discussion in https://core.trac.wordpress.org/ticket/12009 (for the problems I mentioned above), but rather a more holistic rethink of how JS loading should work in WordPress in today's web, along the lines of what's being explored in WordPress/gutenberg#36716. The resulting API would in my mind exist in parallel to the enqueue system, for developers to move to at their own pace. The proposed toggle is meant as an experimental, transitional, best-effort thing that tries to improve performance but may break your site, in the same line as what plugins like This may make some of its limitations acceptable, but it may also be that this solution is simply too hacky and unstable to even exist as an experimental toggle in the performance plugin, and we should just abandon the idea. I'm sensing you're both leaning towards the latter opinion? 🙂 |
I changed the title and added some extra text to the "A possible general solution" section, to emphasise that this is not meant as anything other than an experimental option in the performance plugin, with no goal of it being promoted to Core. |
@sgomes if it's ok with you I've added the 'Needs Discussion' label as it looks like there's still suggestions on approaches that people could weigh in on :) |
Thank you, @eclarke1, that sounds like the right call! 👍 Sorry for not adding that myself, still getting used to the process here 🙂 |
Following the discussion in Slack around the expectations for performance plugin modules, it was made clear that the approach I outline here is not suitable for inclusion in the performance plugin, because it's not solid enough to ever be promoted to WordPress core. The approach of forcefully deferring scripts is fundamentally flawed, in that there will always be scripts that will break, since they weren't written with these conditions in mind. The alternative approach of adding I'm reluctant to close this issue since it's still under discussion, but it sounds to me we shouldn't pursue this particular approach any further. |
It's been nearly a week and there has been no further discussion, so I'm closing this issue. If anyone feels strongly about this and thinks they have a good solution to making this stable enough for an eventual inclusion in WordPress core, please feel free to reopen! |
Catching up with the discussion here, I love this overall idea and think we can at the very least enable the |
I just came across this, and I want to throw a hacky idea here.
What if we can defer inline scripts? We can turn this: <script>
doSomething();
</script> into: <script defer src="data:text/javascript,doSomething();"></script> Technically this should work but could break if the client has set up some aggressive CSP rules. We need to look into the security side of this if we ever want to enable this. Some possible other solutions include doing the similar thing like above but creating some kind of an internal proxy to serve the inline scripts somewhere on the host. This solution is probably too hacky though and I don't think it'd go anywhere, but just leaving this here to possibly spark some more ideas and/or inspire other approaches. |
Hey @kevin940726 thanks for sharing your ideas. Our eventual goal will be to get developers to change their behavior, use script apis vs directly injecting scripts, and expand those APIs to enable defer, and finally encourage its use through things like linter warnings. I still think it is worth considering a performance mode for the plugin (and maybe core) that does more aggressive interventions such as converting/deferring inline scripts automatically as you suggest. |
Following here because not being able to |
Hi @tillkruss - thanks for the report, we are tracking this work in #168 now. |
Background
wp_enqueue_script
and friends are the recommended way of loading JavaScript in WordPress. Besides the loading aspect itself, these methods manage dependencies so that scripts are loaded in the correct sequence, with dependencies executing before their dependants, provided they're correctly declared.The mechanism through which this is accomplished on the browser is the use of blocking scripts:
Blocking scripts are problematic for performance, because they block document parsing and rendering when they're reached. This means that if a blocking script is in
<head>
, for example, the document will remain completely unrendered while the script is fetched, parsed, compiled, and executed, likely resulting in worse scores in metrics like First Paint, First Contentful Paint, and Largest Contentful Paint.One way of avoiding some of these issues is to move the scripts to the footer, which can be done with the
$in_footer
parameter inwp_enqueue_script
. However, this is not an ideal solution from a performance point of view, since from an API perspective it can lead to problems if dependencies and dependants have different values and expectations for$in_footer
.Using the
defer
attributeWhile the enqueue mechanism uses the default behaviour blocking scripts, two other timing options exist on the web: deferred scripts and async scripts. Async scripts execute as soon as they are fetched, thus not maintaining ordering, which means they're unfit for the purpose of managed dependencies (and exhibit generally unpredictable performance). Deferred scripts, however, wait until the
DOMContentLoaded
event to execute, and maintain ordering amongst themselves, thus offering good default performance characteristics (by giving the document a chance to render) while being suitable for a dependency management mechanism.Deferred scripts are usually created with the
defer
attribute:Since modifying the script tags inserted by
wp_enqueue_script
is somewhat difficult, developers don't generally do this themselves, with plugins such asperfmatters
attempting to do this automatically for the user.Challenges with deferring scripts
The biggest issue with deferring scripts is that the
defer
attribute only works with external scripts. Inline scripts ignore the attribute and thus behave as blocking scripts, executing earlier. The following code would thus fail, with the inline code executing before the dependency:Until somewhat recently, the only practical ways around this would have been to either keep everything as external scripts (which would have been problematic for internationalisation or any other scripts that have conditional content on the server side); or instead to register event listeners that would have waited for the dependency to load, and then wrap the inline code in a function that would have waited to be called (which would have been challenging, because it means modifying the inline scripts).
However, with the advent of ES modules and
type="module"
, we can make use of a side-effect of these new features.Module scripts
Realising that blocking scripts are a bad performance default, module scripts were standardised to use deferred semantics instead, both when loaded externally and when inlined. This means that even though module scripts were not created with the purpose of allowing for deferred inline scripts, we can take advantage of that standardisation choice in order to enable that use case.
This means that we could now have a full, working mechanism to enable deferring both external and inline scripts, while preserving ordering:
However, module scripts come with a few additional restrictions, beyond the use of deferred semantics:
defer
for that.this
evaluates toundefined
instead ofwindow
. This is likely not a problem for most scripts, although it should be noted.'use strict';
to be executed in strict mode, module scripts are always in strict mode. Some scripts will break when forced to use strict mode.window
. This means that if a script wants to create a new global, it needs to explicitly dowindow.foo = 'bar'
rather than simplyvar foo = 'bar'
at the top level.A possible general solution
Taking all of the above into account, a possible solution that would work for most use-cases would be to:
defer
attribute to every script enqueued viawp_enqueue_script
type="module"
to every script inlined bywp_localize_script
,wp_add_inline_script
, or any other core method that adds an inline script to the document.In modern browsers, this would generally only break in situations where an inlined script doesn't work correctly in strict mode, when it's expecting top-level
this
to evaluate towindow
, or when it's expecting to set globals with a top-level variable declaration. I expect this to be a reasonable opt-in compromise.This could be implemented as a simple toggle in the performance plugin, which the user could optionally enable, modifying the core enqueuing behaviour as a whole. If the user ran into one of the compatibility issues above, or if they intend to keep supporting old browsers on a best-effort basis, they could simply keep the switch toggled off.
Edit: to clarify, this toggle is not meant as a general, long-term solution, nor something that would eventually be promoted to Core. It would simply exist as an experimental option in the performance plugin, that may well improve performance for a site, but may also break it as a result.
Safeguard considerations
In order to avoid accidentally breaking admin pages in case a compatibility issue exists, we could ensure that admin pages were always excluded from this optimisation, and thus kept their existing blocking behaviour, particularly since performance optimisation is not as important there.
Another option would be to have a global safety URL parameter that would disable the optimisation (e.g.
disable_js_defer
), although this would be much less discoverable and hard to stumble upon as a solution when your site is broken. It may be a good complement to the above, however, and particularly useful for debugging compatibility problems.Reference and thanks
<script defer>
MDN Web Docs<script type=module>
support, Can I UseThank you to Khoi Pro for suggesting looking into this as part of our JS issue prioritisation!
This issue covers the card in https://github.com/WordPress/performance/projects/3#card-75561256.
The text was updated successfully, but these errors were encountered: