AssetManager creates different hash for same pipelined files when extra asset is added to before/after position

[pamtbaau]

Grav version: grav-admin-v1.7.0-rc.2

Problem: AssetManager creates different hashes for pipelined files when extra asset is added in 'before' or 'after' position. The contents of the different pipelined files are exactly the same though.

Expected result: Assets in position before/after should not change hash of pipelined assets.

Example:

I have the following in base.html.twig:

{% block javascripts %}
   {% do assets.addJs('https://code.jquery.com/jquery-3.4.1.min.js', { priority: 100, loading: 'defer' }) %}
   {% do assets.addJs('theme://js/popper.min.js', { priority: 98, loading: 'defer' }) %}
   {% do assets.addJs('theme://js/bootstrap.min.js', { priority: 97, loading: 'defer' }) %}
 {% endblock %}

{{ assets.js('head', {loading: 'defer'}) }}

The output is:

<script src="/grav/site-base/assets/3c4701a9a8e3c486b719e9d09e64a839.js" defer></script>

On some pages I add a plugin. My plugins add assets as follows:

$this->grav['assets']->addJs("plugin://contactform/js/contactform.min.js", [
   'group' => 'head',
   'position' => 'after',
   'loading' => 'defer',
]);

The output is:

<script src="/grav/site-base/assets/24180c8c0af4f6c7bc1b1f1f95235033.js" defer></script>
<script src="/grav/site-base/user/plugins/contactform/js/contactform.min.js" defer></script>

The contents of '3c4701a9a8e3c486b719e9d09e64a839.js' and '24180c8c0af4f6c7bc1b1f1f95235033.js' are according to diff exactly the same.

[rhukster]

Ordering can have a profound impact on both CSS and JS. If you had an issue with a particular order, and you fixed it by changing the order, your browser would not pick up the change if the hash remained the same.

The real issue is why your asset that is 'after' the pipelined assets, affecting the hash. This is probably because the 'before' and 'after' assets should be skipped in hash calculation.

[pamtbaau]

What do you mean by 'changing order':

1. when asset gets moved from 'before' to 'after' position, or
2. that the value of 'priority' within pipelined group changes?

In case of 2. the hash should definitely change. In case 1. I'm not sure yet...

Yes, the issue I'm referring to is that after/before assets should not impact the hash.

[pamtbaau]

Did some debugging and found that in Assets::addType(), the order ($options['order']) of each added asset is set using the count of assets already added. This ordering does not take into account the new asset's group and position.

Hence, if on some page a plugin adds an extra asset in the before/after position, assets added later in the process receive a different 'order' value.

Since the 'order' is part of the value on which the hash for 'pipelined' assets is calculated, the hash will change.

Problem: This will force the browser to fetch the new pipelined asset file, only because the hash has changed while its content remained exactly the same.

Possible solution: I cannot oversee all implications, but a solution that works for me is to generate an 'order' value per type, group and position combination:

• Add class variable to class Assets

  /** @var array to contain next 'order' value per asset 'type', 'group' and 'position' combination */
  protected $order = [];

• In Assets::addType(), replace:

  $options['order'] = \count($this->$collection);

  with:

  $group = $options['group'] ?? 'head';
  $position = $options['position'] ?? 'pipeline';
  
  if (!isset($this->order[$type][$group][$position])) {
   $this->order[$type][$group][$position] = 0;
  }
  
  $options['order'] = $this->order[$type][$group][$position]++;

Above code prevents Grav to generate a new asset file (with exact same content already loaded) to be loaded by the browser when a plugin adds assets in the before/after position.

[pamtbaau]

Any progress on this?

[rhukster]

Afraid not yet.

[NicoHood]

I also ran into the same issue. I inspected the code and the patch provided by @pamtbaau . I think the code is the best solution you can come up with, I found no better option. I've opened a PR with the code from @pamtbaau , hope that is okay for you.

https://github.com/getgrav/grav/pull/3122

[pamtbaau]

@NicoHood, That's fine with me... And thanks for picking this up.