Automattic / page-optimize

WordPress plugin to optimize pages for faster load and render in the browser
GNU General Public License v2.0
31 stars 11 forks source link

apply_filters( 'script_loader_tag' ) does not fire #48

Open paulschreiber opened 4 years ago

paulschreiber commented 4 years ago

In concat-js.php, in do_items(), you have this check:

if ( is_array( $js_array['handles'] ) && count( $js_array['handles'] ) === 1 ) {

This condition never seems to be true, and the filter never fires.

When I am logged in, handles contains four items:

array (
  0 => 'hoverintent-js',
  1 => 'admin-bar',
  2 => 'main',
  3 => 'wp-embed',
)

When I am not logged in, handles contains two items:

array (
  0 => 'main',
  1 => 'wp-embed',
)
aidvu commented 4 years ago

@brandonpayton could you chime in here? What was the case for === 1?

brandonpayton commented 4 years ago

Hi @paulschreiber,

Page Optimize had a bug where page-optimize was preventing the twentytwenty theme from adding async/defer attributes, and the === 1 case was part of the solution.

While attempting to concatenate scripts, sometimes we find a script that cannot be concatenated due to having inline scripts attached. We cannot concatenate static script content when dynamic inline script content should be inserted in between scripts or we will break the order of execution. So whenever we encounter a script that has attached inline script, we stop concatenating before that script and treat it like a standalone script. Then we try to start a new concatenation of the subsequent scripts. This need to break concatenation into different groups sometimes leads to a "group" of one script.

For example, let's say we have scripts A, B, C, and D. Script C has dynamically generated inline script and cannot be concatenated with other scripts. Page Optimize will mark the scripts like:

When Page Optimize finds D by itself in a concat group, the === 1 case tells it to treat D like a standalone script since there is no gain in concatenating a single script.

Whether you encounter this group-of-one situation depends on what plugins you have installed and what script they are enqueuing for the current front end content.

Does this make sense? Does it answer your question?

paulschreiber commented 4 years ago

That's helpful. It would be good to rewrite the comment to more clearly explain what situations you expect the filter call to fire, and provide a page_optimize_site_loader_tag filter that you can apply regardless of the number of scripts.

brandonpayton commented 4 years ago

That's helpful. It would be good to rewrite the comment to more clearly explain what situations you expect the filter call to fire

In this case, it's simply meant to have the same meaning as script_loader_tag in core. It's not meant to be a plugin feature people know about but more like a core behavior we try to avoid breaking. We only intend for Page Optimize to deviate from core script-printing behavior when it is combining multiple scripts into a single script tag.

and provide a page_optimize_site_loader_tag filter that you can apply regardless of the number of scripts.

In most cases, when a script cannot be concatenated, we delegate script printing to core WordPress. It's only when we have concatenate-able scripts that we handle printing ourselves. What sort of use case do you imagine for a page_optimize_site_loader_tag filter?

paulschreiber commented 3 years ago

Typically I use these filters for adjusting defer/async behaviour for JS and deferring CSS via <link rel="preload">.