Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.0.0 - changes to the way the plugin works #147

Closed
benhoIIand opened this issue Aug 9, 2015 · 15 comments
Closed

v1.0.0 - changes to the way the plugin works #147

benhoIIand opened this issue Aug 9, 2015 · 15 comments
Milestone

Comments

@benhoIIand
Copy link
Owner

My proposal is to change the plugin to work in a slightly different way. Currently the plugin's entry point is a file (generally a HTML file) where it would parse the content looking for assets to bust. This restricts the plugin to things that it can parse, so in the case of JS files, we'd have to add a new parser for that. It also means that other languages, PHP being one, can't be handled elegantly.

Solution
The solution to these problems will be to reverse the order of actions; the new entry point being a folder of assets, busting them and then going through each file that you wish to bust and replacing references. This should clean up a lot of the code and remove a lot of the slightly finicky regex rules in there. Some options can also be removed with this approach, enableUrlFragments being one of them.

I've created this issue incase anyone else has ideas about how to make the plugin better!

Edit
Latest code and implementation can be found in the v1.0.0-beta branch. If anyone wishes to test on this branch and change their workflow to the new method, that would be stellar!

@benhoIIand benhoIIand added this to the v1.0.0 milestone Aug 9, 2015
@benhoIIand
Copy link
Owner Author

With this change, it's becoming clear that some of the current options and features will be remove. I will keep this list up to date:

  • Busting already cached assets - this was pretty hacky in the first place as it's very difficult to detect a hash in the file. Due to the new method of gathering the list of files to be hashed, it's going to be encouraged/enforced that you have a clean workspace before every build. More information can be provided on this if needed.
  • Overriding the base directory - all assets will be discovered first, so the ability to modify this map of assets will be provided, meaning that the overriding of the baseDir can be done there
  • cdnPath option - this will no longer be needed as the files that'll be hashed have already been discovered. Works out the box now :)
  • enableUrlFragments/deleteUrlFragments - these are obsolete
  • rename - this was turned on by default a while back. Going forward, this will be the only option for busting files. The idea of this plugin is to enable users the best chance to have their assets cached forever and IMO, providing a brand new file with a new name is the best way.
  • replaceTerms- as with overriding the baseDir, this can be done in some kind of modify function
  • filters - another obsolete option as we won't need to filter out the assets found in markup anymore
  • ignorePatterns - again, it's obsolete option. This should be done by grunt anyway and not in the plugin

@tusbar
Copy link

tusbar commented Aug 16, 2015

Hey @HollandBen,

I’ve been testing the new branch. This is a good change.

A few issues:

  • I have a CSS file that includes both fontawesome.woff and fontawesome.woff2. When replacing in file, markup.split(original) will match fontawesome.woff2 when looking for fontawesome.woff and it will insert the incorrect hash.
  • It would be nice if it would only bust files that are actually being used. Right now it busts all the files matched by assets.

@tusbar tusbar mentioned this issue Aug 16, 2015
2 tasks
@benhoIIand
Copy link
Owner Author

Hey, thanks for testing the next version 👍

It would be nice if it would only bust files that are actually being used

Can you explain why this is? The current version of this plugin follows this pattern of finding an asset and then busting it, but with the new approach, all assets are busted and then references are updated. The main problem I can see people having with this is that it will created a lot of noise with new files being created all the time. My response to that would be to have your build pipeline setup in a way that each run of this task is done against a new, clean file system. The responsibility of this plugin is to just bust assets, not to clean up after or deal with files that have already been hashed.

@tusbar
Copy link

tusbar commented Aug 16, 2015

@HollandBen here’s an example:

{
     options: {
        baseDir: 'public',
        assets: 'assets/*',
        deleteOriginals: true
    },
    files: [
        'templates/only-one-template.jade'
    ]
}

This will rename all the files in assets, though it will only change the include paths from templates/only-one-template.jade.

The others assets – that are not used by that template will be renamed as well. Thus the other templates will fail including the referenced asset files.

@benhoIIand
Copy link
Owner Author

I've been thinking about removing the deleteOriginals option as well. Again, it comes back to how the build pipeline is setup, but it should follow a procedure or copy -> process -> deploy, that way the copy part is in a clean setup and the original files aren't changed.

@tusbar
Copy link

tusbar commented Aug 16, 2015

@HollandBen fair enough. I’ll update the PR to only include the two fixes.

@benhoIIand
Copy link
Owner Author

I'm looking to ship this after next week - finally!!

@benhoIIand
Copy link
Owner Author

I've created a new 1.0.0-rc branch which is the release candidate for version 1. I'm exceptionally busy next week so if people wish to start using it, that'd be fantastic!!

You can install the beta using npm install --save-dev grunt-cache-bust@beta

@camwes
Copy link

camwes commented Jan 20, 2016

@HollandBen I am unable to get this to work for my project. I get the error:

Warning: Cannot read property 'indexOf' of undefined Use --force to continue.
TypeError: Cannot read property 'indexOf' of undefined
    at /Users/camwes/dock/projects/jukeboxx/node_modules/grunt/lib/grunt/file.js:55:28
    at Array.forEach (native)
    at processPatterns (/Users/camwes/dock/projects/jukeboxx/node_modules/grunt/lib/grunt/file.js:53:34)
    at Object.file.expand (/Users/camwes/dock/projects/jukeboxx/node_modules/grunt/lib/grunt/file.js:110:17)
    at Object.<anonymous> (/Users/camwes/dock/projects/jukeboxx/node_modules/grunt-cache-bust/tasks/cachebust.js:32:14)
    at Object.<anonymous> (/Users/camwes/dock/projects/jukeboxx/node_modules/grunt/lib/grunt/task.js:264:15)
    at Object.thisTask.fn (/Users/camwes/dock/projects/jukeboxx/node_modul

I am using Grunt 0.4.5 and Node 0.12.7. My gruntfile looks like this:

    cacheBust: {
      assets: {
        files: [{
          expand: true,
          cwd: 'client/js',
          src: ['*.js'],
          dest: 'test/js'
        }]
      }
    },

@benhoIIand
Copy link
Owner Author

Hi @camwes. Is this using the beta version of the plugin? I can see from your configuration that you are only telling the plugin where to find the assets, but not telling it which files need to have their references updated.

@camwes
Copy link

camwes commented Jan 25, 2016

@HollandBen I am using the beta version. It is not very clear from the docs and examples what the difference is. Are you saying that the files attribute only determines where the files can be found. If so, how do I choose which files need to have their references updated?

@camwes
Copy link

camwes commented Jan 28, 2016

Was able to get this working after thinking a little more abut your advice. Working version looked like this:

    cacheBust: {
      prod: {
        options: {
          assets:  [
            'static/assets/dist/js/index.js',
          ]
        },
        files: [{
          expand: true,
          deleteOriginals: false,
          cwd: 'static/assets/dist/js',
          src: [
            'index.js'
          ],
        }]
      }
    },

This is a agreat improvement on the plugin and I look forward to testing it some more.

@benhoIIand
Copy link
Owner Author

Glad you got it working @camwes. Can you elaborate on what wasn't clear in the documentation? I've changed the Overview section this morning to hopefully explain the basic configuration better.

@benhoIIand
Copy link
Owner Author

Moving the default package on npm to be v1.0.0

@MaqSaid
Copy link

MaqSaid commented Oct 5, 2018

I have 2 questions for you 1) Can your following code take scr= ['Index.cshtml'] and how will you implement to get .less and other asset files from angular 4. Thanks.
cacheBust: {
taskName: {
options: {
assets: ['assets/**']
},
src: ['index.html']
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants