clubstudio / craft-asset-rev

A Craft CMS plugin to help with cache busting
MIT License
109 stars 7 forks source link

Feature Request: more flexibility for the fallback #11

Closed jan-dh closed 7 years ago

jan-dh commented 7 years ago

I'm using a Webpack flow for my assets and my manifest.json looks something like this:

{
    "app.css": "css/app.a7ebf63.css",
    "app.js": "js/app.a7ebf63.js"
}

If I'm not in production however, the manifest.json is not being generated and I would like to be able to fallback to the normal files: css/app.css and js/app.js. I'm using this in combination with load.js.

For the production part I'm using the following

{% set css = rev('app.css')|default('css/app.css') %}
<script type="text/javascript">
      loadjs('{{css}}', {
          success: function() {
          console.log('css loaded');
      }
    });
 </script>

However, if the manifest.json does not exist, instead of falling back to the default I specified it tries to append a date string like specified in the docs. This way, the css folder gets ignored and I get an error that the css file is not found. It would be nice to have some flexibility in the fallback, maybe disable it, so you can handle it a bit more flexible by, for example, giving it your own default fallback.

scottwakefield commented 7 years ago

Hey @jan-dh,

Sure, I understand. I've been thinking about this and have started playing around with the idea of making the revving options, including fallbacks, much more flexible.

I'm currently working towards updating the plugin so something like this is possible in the configuration file:

'strategies' => [
    'manifest' => \AssetRev\Utilities\Strategies\ManifestFileStrategy::class,
    'querystring' => \AssetRev\Utilities\Strategies\QueryStringStrategy::class,
    'fallback' => function ($filename, $config) {
        return $filename;
    }
],

'pipeline' => 'manifest|querystring|fallback',

Meaning you'd have full control over which revving strategies where used and also the order in which they were attempted. Supporting closures here would allow you to easily define your own fallback functionality.

In the example above, the plugin is configured to try and use a manifest file first, then a timestamp query string, but will fallback to the closure and just returning the filename if neither of those is possible.

Overall I think that would give everyone the flexibility to adapt the plugin to their individual setups. What do you think? Would that work for you?

jan-dh commented 7 years ago

@scottwakefield that would definitely work. Looking forward to trying it out.

scottwakefield commented 7 years ago

@jan-dh – there's still some work to do, but perhaps give this beta version a try: https://github.com/clubstudioltd/craft-asset-rev/archive/b48f6047c7b30a638c5edd1c779744b770e36a96.zip

Check the updated config.php file for new configuration options.

You can track progress on the feature/configurable-strategies branch.

jan-dh commented 7 years ago

Very flexible, I like it!

scottwakefield commented 7 years ago

@jan-dh Great stuff. I've got to write some updated tests, but once they're done I'll merge into master and tag a new release.

Thanks again for opening the initial issue – this update should knock a couple of other issues on the head at the same time 👍