Add Core's script_loader_tag filter for scripts #62
Conversation
| * @param string $js_array The array with the type, path, and handles for the scripts being concatenated. | ||
| * @param string $href The script's source URL. | ||
| */ | ||
| $tag = apply_filters( 'script_loader_tag', $tag, $js_array, $href ); |
There was a problem hiding this comment.
The filter's $handle is expected to be a string, but here it is supplied an array. If the common use-case is to modify the tag regardless of what is being concatenated then specifying a static handle would be better. Might the $js_array be passed in as an unnamed final attribute for the cases where what is being concatenated is expected to affect the tag's output?
Should we could consider a custom filter here since this is outside the expected enqueuing context of the script_loader_tag filter, even though it serves the same purpose?
There was a problem hiding this comment.
My only worry is that then we won't take advantage of all the themes/plugins that use this filter for this purpose?
I went this route as Atomic on WPCOM runs the same thing: https://github.com/Automattic/page-optimize/blob/master/concat-js.php#L263
Problem
There is no way to filter our concatenated scripts and add things like
async,defer, or other valid script attributes.@rinatkhaziev had a good suggestion in this PR: #38
I actually worked based on that to start and had a WIP based on that code, but then realized that Core already has this filtering in place with
script_loader_tag, and in fact plugins and themes rely on this script to be able to do things like this.Here's an example of Twentytwenty and how it uses this filter to add
asyncanddefer:https://github.com/WordPress/twentytwenty/blob/6d0a5240af108d02f58ec797fd49b32458a4c698/classes/class-twentytwenty-script-loader.php#L31
I think instead of adding a new filter, bringing in what Core already does into this would be better because it has more support from things already being used.