Automatically exclude assets from popular caching/optimizer plugins

[inpsyde-maticluznar]

A common nuisance in supporting and maintaining plugins is that performance optimization plugins tend to be quite aggressive about optimizing CSS/JS assets which often leads to broken functionality without immediate wrongdoing on our side. Examples include:

• Minimizing/uglifying assets
• Merging all individual assets into one monolith

Usually, this is undesired and leads to complications which the site admins need to sort out themselves. It leads to support tickets on our side and frustration on end-user-side.

However, a few of these plugins offer an API to exclude individual assets from being processed. In the past, we had to implement these blacklists on a per-plugin basis. But thanks to the increasing adoption of this library, we are now in a position to introduce a common entrypoint for this kind of configuration. So we would like to implement new functionality that makes exclusion of our assets a straightforward ting.

This PR adds an IgnoreCacheHandler. IgnoreCacheHandler applies per-plugin specific logic to exclude plugin files from minifying and combining. Currently, two WordPress plugins are supported:

• Siteground caching plugin
• W3TotalCache plugin

Usage:

$assetManager->register(
    new Script('assets-plugin-script', plugin_dir_url(__FILE__) . 'resources/test-script.js'),
    new Style('assets-plugin-style', plugin_dir_url(__FILE__) . 'resources/test-style.css')
);

$assetManager->ignoreCache();

[codecov]

Codecov Report

Attention: Patch coverage is 82.14286% with 15 lines in your changes missing coverage. Please review.

Project coverage is 90.53%. Comparing base (8bd2a3c) to head (ad7026e).

Files	Patch %	Lines
src/Util/AssetPathResolver.php	16.66%	5 Missing ⚠️
src/Caching/IgnoreSitegroundCache.php	80.00%	4 Missing ⚠️
src/AssetManager.php	71.42%	2 Missing ⚠️
src/Caching/IgnoreW3TotalCache.php	85.71%	2 Missing ⚠️
src/Caching/IgnoreCacheHandler.php	95.65%	1 Missing ⚠️
src/Loader/AbstractWebpackLoader.php	88.88%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master      #54      +/-   ##
============================================
- Coverage     91.02%   90.53%   -0.50%     
- Complexity      282      304      +22     
============================================
  Files            21       24       +3     
  Lines           836      898      +62     
============================================
+ Hits            761      813      +52     
- Misses           75       85      +10     

Flag	Coverage Δ
unittests	90.53% <82.14%> (-0.50%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Chrico]

While I understand the concept, I kind of don't like that with that PR also code which is not affected by the PR is changed (psalm/code style).

Regarding the topic itself: Shouldn't we move the "ignoreCache" to the Asset, to allow deciding which assets are cacheable and which are not? Currently you can just enable/disable it for all, but not for specific ones.

[inpsyde-maticluznar]

While I understand the concept, I kind of don't like that with that PR also code which is not affected by the PR is changed (psalm/code style).

Regarding the topic itself: Shouldn't we move the "ignoreCache" to the Asset, to allow deciding which assets are cacheable and which are not? Currently you can just enable/disable it for all, but not for specific ones.

For the first part, I would like @Biont to jump in. I honestly don't know what to do when I work on packages that have errors in code style - fix it or leave it :) It looks like the opinions between individuals vary.

For the second part, I will also ask @Biont to jump in. Should the functionality be applied globally or individually?

[Biont]

I can get behind the notion that the psalm fixed potentially cloud the view on the functional changes. I guess it was done here because it feels bad to publish a PR with broken CI - even if the problems are not introduced by new code.
Fixing psalm in a separate PR, then builing on top of it is a way to avoid the situation.
@Chrico Do you want changes to be made or is this more of a nudge for the future?

---

Moving the exclusion to the actual asset sounds reasonable.

• It offers more control - for example when you are fine with getting your 5mb CSS file optimized, but cannot allow third-parties to uglify your critical JS SDK integration.
• It stays out of trouble when the library is used at website-level ( -> one instance across many plugins). In this scenario, one plugin would be able to make a quite bold choice that then also applies to other, entirely unrelated packages.
• ...and the only cost is slightly more effort in case you actually want to enforce exclusion for all assets under your control. But this is comparably easy to pull off with other means at project level (eg. apply_filters or centrally configuring the state in another service )