search

benhoIIand / grunt-cache-bust

Cache bust static assets using content hashing
MIT License
265 stars 103 forks source link

Add option to use a URL parameter for cache-busting instead of copying files #185

Closed PLPeeters closed 8 years ago

PLPeeters commented 8 years ago

This PR adds an option named useUrlParam that makes it possible to cache-bust using a URL parameter instead of copying files to add a version number directly to the filename. It replaces references like /path/to/asset.js with /path/to/asset.js?cache-bust=HASH without touching the file.

benhoIIand commented 8 years ago

Hi @PLPeeters. The recent bump to version 1.x.x has removed the functionality for cache busting using a query string. Have a read through the documentation and issue #147 for more info

PLPeeters commented 8 years ago

Hi,

Reading through the issue, I see why it would make sense to prefer cache busting by rewriting files entirely. However, in my case, my build process is set up in a way that prevents me from using that method.

I would change my build process entirely if I could, but that's something I can't do for now, because it would require changing the server config for multiple projects.

In the end, why not leave the choice of the cache busting method up to the end user? I totally see why you wouldn't want to bloat the code with unnecessary options, but I think this one makes sense and could be a relatively common use case.

Anyway, I've made my case, the rest is up to you :)

benhoIIand commented 8 years ago

I'm not against giving people the option. With the new release I wanted to trim the fat and then see what people noticed was missing. I've left some comments so after they're done I'm happy to add the feature back in

PLPeeters commented 8 years ago

Sure thing. I hesitated regarding the named parameter too, guess I'll just leave the hash then.

benhoIIand commented 8 years ago

A few more things to do:

PLPeeters commented 8 years ago

On it.

PLPeeters commented 8 years ago

@hollandben Any particular reason this hasn't been merged yet? :)

benhoIIand commented 8 years ago

Nope - I hadn't seen/been notified of the commits tbh. Will have a look shortly

PLPeeters commented 8 years ago

I guessed that might have been the case, hence my comment :)

benhoIIand commented 8 years ago

Nice - thanks

benhoIIand commented 8 years ago

Have publish v1.2.0 - I bumped it a minor version as it was a new feature for this major release