Add async defer filter #36
Conversation
Some sites might want to add async and defer to the concatenated javascript, this can now be turned on by returning true (or a truthy value) to the 'js_concat_should_async' filter
|
|
Allow to enable both or either of async and defer to the concatenated script.
|
Thinking a bit more, this feels pretty dangerous since you have to set aync/defer to all scripts (hard to know from the proposed filter what scripts a bundle includes), which could break things (e.g. inline scripts that need an enqueued script like jQuery). A better approach might be to skip concatenation for any scripts that need async or defer. Or introduce a way to concatenate the async and/or deferred files together. |
|
I agree it could be expanded upon but I think it allows some extra flexibility and while it might be nice to concatenate scripts in different buckets I think we can build that functionality if / when we need it. |
|
We are doing exactly what Mo says, skipping concatenation, for those that we need to defer for performance reasons currently. If the concatenation script could create buckets of concatenated scripts, that would be ideal. Having a potential for four buckets/minimized scripts doesn't sound optimal, but I think it is the only way to safely have these features AND auto-joining. In short, we do need that functionality now. |
|
@mjangda the defer maintains the order of execution, so as long as you're not enqueueing a non-deferred JS asset which has a dependency an asset that you're deferring, that should be fine. I think the ones using such advanced features should know what they are doing, and understanding how deferring and dependencies work. And I'm much happier with deferring the concatenated asset rather than split that file up into 15 pieces, mostly because gzip has a better compression ratio on larger files compared to multiple smaller ones. Speaking of different buckets for asynced / deferred scripts, I've already looked into Issue #33 , but this seems to touch on a larger scope that something we can patch just in the plug-in, but rather something which touches the WP Core script enqueueing and the creation of new groups of scripts to get it to work in a clean fashion. |
That's my concern as well. If we implement buckets this seems to re-implement what core is/should be doing. @brianlayman How would you envision this working? |
|
Personally I feel that this feature is helpful as it is. It brings to the table a possibility to do this type of performance improvements now, without relying on anything in Wordpress Core. I'd definitely use it for the sites I'm working on and would stop deactivating the js_concat feature because that just turns any script in a render-blocking one, even if it's placed in the footer. |
|
I agree with @mjangda, as-is this is a bit dangerous. For it to be useful as a short-term fix, I think additional arguments need to be passed to the filters. Perhaps One additional suggestion, this could be made more generic and flexible by making the filter |
|
I second what @mboynes said. having async/defer filter is nice but there are other valid attributes that would be nice to have. (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script). @sboisvert I can open another PR with a more generic approach (e.g. have a filter that controls all extra attributes). |
|
@rinatkhaziev sounds good, let's do that. |
…ag attributes, the filter gets passed an empty array and has to return an associative or regular array with attributes as keys, additional arguments for the filter are: $href - current batch url, $js_array - additional data for current batch of scripts. This filter runs on each batch of files; extra arguments are needed to better control attributes being assigned to a resulting tag. For example, we want to set async/defer for either a specific group of files being concatenated or maybe we want to do a blanket change for every resulting script tag. Fixes Automattic#33, supersedes Automattic#36
|
Seems to have been done in #64 |
Some sites might want to add async and defer to the concatenated javascript, this can now be turned on by returning true (or a truthy value) to the 'js_concat_should_async' filter