teamcfadvance / cfstatic

CfStatic is a framework for managing the inclusion and packaging of CSS and JavaScript in CFML applications.
https://teamcfadvance.github.io/cfstatic
MIT License
102 stars 35 forks source link

addCacheBusters = false seems to not be working? #87

Closed ghost closed 11 years ago

ghost commented 11 years ago

Hey Dominic,

I'm trying to turn off the cache busters in my resulting compiled CSS files (which originate as LESS files) but I can't seem to get cfStatic to listen to the addCacheBusters parameter in init. Here's my code:

    #createObject("component","org.cfstatic.CfStatic")
        .init(
            staticDirectory = ExpandPath($.siteConfig("themeIncludePath")),
            staticUrl = replace($.globalConfig("context") & right(ExpandPath($.siteConfig("themeIncludePath")),len(ExpandPath($.siteConfig("themeIncludePath")))-len(expandPath("/murawrm"))), "\","/","all"),
            outputDirectory = "compiled",
            minifyMode = "package",
            checkForUpdates = true,
            javaLoaderScope = application.configBean.getValue('cfStaticJavaLoaderScope'),
            addCachBusters = false
        )
        .include('/css/core/')
        .include("/css/ie/ie8/")
        .renderIncludes('css')#

Am I doing something wrong here?

Thanks!

ghost commented 11 years ago

Maybe I'm missing the purpose, here. The resulting files don't have the timestamp in their actual filenames - but image references INSIDE those files do. I'm trying to address the latter - i want caching to work on images.

ghost commented 11 years ago

I think I'm a complete newb when it comes to github and branching/pull requests - but here's some proposed changes that seem to be working for me... would you mind taking a look and seeing if that can be pulled into your core?

https://github.com/beamerweb/cfstatic/commit/252175a795e58fe6888a306dd1a14f84c3ad4f8a

Thanks!

DominicWatson commented 11 years ago

I'll take a look ASAP at the changes, the code should work as advertised. However, the cache buster should only change when the image changes so I wouldn't advise turning the feature off (unless it isn't working) On 17 Sep 2013 01:06, "Jason Beam" notifications@github.com wrote:

I think I'm a complete newb when it comes to github and branching/pull requests - but here's some proposed changes that seem to be working for me... would you mind taking a look and seeing if that can be pulled into your core?

beamerweb@252175ahttps://github.com/beamerweb/cfstatic/commit/252175a795e58fe6888a306dd1a14f84c3ad4f8a

Thanks!

— Reply to this email directly or view it on GitHubhttps://github.com/DominicWatson/cfstatic/issues/87#issuecomment-24555031 .

ghost commented 11 years ago

Hey Dom - Thanks for jumping on this so quick!

I may be missing the point of the cachebuster setting, then. What I'm trying to do is build a preloader of all the images on a page but because there's a number tacked onto all the image urls in the CSS, the preloader is defeated (which I admit may be desirable in many cases). The changes I've made work for what I'm after, but if these changes conflict with what you had in mind for cache busting, maybe I could request a separate setting?

I'm hoping to keep using cfstatic but stay on the upgrade path if at all possible.

Thanks again :)

DominicWatson commented 11 years ago

Cool, I think it makes sense for the setting to turn image cache busters off too, though perhaps a separate setting would be more sensible? I'm presenting on Cfstatic at CfCamp so will be getting to fix some things soon.

Thanks for the feedback, btw. On 17 Sep 2013 15:43, "Jason Beam" notifications@github.com wrote:

Hey Dom - Thanks for jumping on this so quick!

I may be missing the point of the cachebuster setting, then. But what I'm trying to do is build a preloader of all the images on a page but because there's a number tacked onto all the image urls in the CSS, the preloader is defeated (which I admit may be desirable in many cases). The changes I've made work for what I'm after, but if these changes conflict with what you had in mind for cache busting, maybe I could request a separate setting?

I'm hoping to keep using cfstatic but stay on the upgrade path if at all possible.

Thanks again :)

— Reply to this email directly or view it on GitHubhttps://github.com/DominicWatson/cfstatic/issues/87#issuecomment-24593514 .

ghost commented 11 years ago

I don't want to presume anything but in my mind, the idea of turning off cache busters would include this. I'm trying to envision a situation where i'd want them on the images and NOT on the CSS or JS files. I think the concept of cache busting is more thorough and can't imagine a case where i'd want one without the other.

DominicWatson commented 11 years ago

Maybe my misunderstanding, but the purpose you gave for turning them off for images was to allow image preloading - does cache busting the css filenames also break this?

D On 17 Sep 2013 18:21, "Jason Beam" notifications@github.com wrote:

I don't want to presume anything but in my mind, the idea of turning off cache busters would include this. I'm trying to envision a situation where i'd want them on the images and NOT on the CSS or JS files. I think the concept of cache busting is more thorough and can't imagine a case where i'd want one without the other.

— Reply to this email directly or view it on GitHubhttps://github.com/DominicWatson/cfstatic/issues/87#issuecomment-24606175 .

ghost commented 11 years ago

Well in that case, yes - and you know this stuff way better than I do. In my current use case, leaving the numbers on the css filenames wouldn't be a problem for me. It's just that in my mind "do not bust caches" is an all or nothing thing. Though I admit i'm definitely thinking about this from my current use case.

A separate setting is fine, too - it's more work that I'm struggling to identify use cases for ... but that could just be my limited perspective.

DominicWatson commented 11 years ago

My thinking is that you shouldn't need to lose the powerful filename cache busting if you don't need to (it's really great not needing to worry about outdated cached versions of files when you deploy updates). I'll take a look as soon as I can.

Again, thanks for the feedback. On 17 Sep 2013 18:28, "Jason Beam" notifications@github.com wrote:

Well in that case, yes - and you know this stuff way better than I do. In my current use case, leaving the numbers on the css filenames wouldn't be a problem for me. It's just that in my mind "do not bust caches" is an all or nothing thing. Though I admit i'm definitely thinking about this from my current use case.

A separate setting is fine, too - it's more work that I'm struggling to identify use cases for ... but that could just be my limited perspective.

— Reply to this email directly or view it on GitHubhttps://github.com/DominicWatson/cfstatic/issues/87#issuecomment-24606706 .

ghost commented 11 years ago

That's a very logical reasoning ... As long as I can shut off the image one, I'll be happy! For now, I'm running with my tweak - i have a little time before I'll need to go to production so hopefully our timelines meet conveniently :)

Thank you so much for digging into this! :)

DominicWatson commented 11 years ago

This has been implemented in the develop branch ready for upcoming 0.7.0 release. The new option is "addImageCacheBusters".

Closing - please reopen if it does not work for you.

ghost commented 11 years ago

Thanks dom! I think this'll be just fine! :)

ghost commented 10 years ago

Hey dom - i apologize for the delay but i wanted to touch base on this. Mura isn't pulling this commit yet (i think it's because it's not technically released as a milestone yet?) -- do you have an idea of an ETA on 0.7.0 being released?

Thanks!

DominicWatson commented 10 years ago

Hi - once again, apologies for my uber-slow response. I'm going to try to squeeze this release in this month. It gets hard when you're not constantly working on a codebase and haven't organised release notes, etc. properly. Lesson to learn for the next release.