Cowboy-coder / bundle-up

An asset manager for nodejs
MIT License
56 stars 22 forks source link

Added minification support for JavaScript files #4

Closed panosru closed 12 years ago

panosru commented 12 years ago

Added support for minification on javascript files.

Also in Coffee-Script there is no need to have return statement since coffee don't produce any void functions, even transparently they return always something so this:

 method = (a) ->
    return a;

is same with this:

method = (a) ->
    a

you could emulate a void method like this:

method = (a) ->
    a.action() # or what ever here...
    null #return null to emulate a void method
Cowboy-coder commented 12 years ago

Thx man! I will merge it when I got have the time. I just need to make sure to provide good defaults. Because both minification of javascript and css needs to be configurable. Because I'm pretty sure that not everyone will want to have their css and/or javascript minified without having a way to turn it off.

The tough question is though... should minification be default or not? The minification should only happen when bundle:true is set that's for sure, but it needs to be configurable in some way. The reason it shouldn't happen when bundle:false is set is because bundle:false is only for development (because it adds on the fly compiling and it's easier to debug when all files isn't concatenated.) Also... having the minification happen in development is just too slow (at least with uglifyjs). And fast development is important.

panosru commented 12 years ago

I agree with you that the minification should happen only on bundle:true for the reasons you mentioned.

Indeed it should be configurable, what if instead of bundle:true you would have something like this:

bundle : {
    enabled : true,
    hooks : {},
    something_genius : {}
    ....
}
Cowboy-coder commented 12 years ago

I like the idea!

Some tweaks that comes to mind:

BundleUp(app, 'assets.js', {
  staticRoot: __dirname + '/public'
  staticUrlRoot: '/'
  development:true
  bundles: {
    minifyCss:true,
    minifyJs:true
  },
  hooks: {
    preMinify: {
      js: [...]
      css: [...]
    },
    afterMinify: {
      js: [...]
      css: [...]
    }
  }
});

Let me know what you think :)

panosru commented 12 years ago

I like the idea and I agree with what you say! development:true would be better I think against production:false, I don't see any reason to have env : 'development' or what ever so I believe it comes to development:true VS production:false

So I will give the win to development:true for the reasons that first of all, the project goal is to end up in production as a result this module will work for production in the end so the goal is to have this module in production so we end up that the default behavior of the module is for production. During the way to production we are in development/testing mode as a result this is something temporary so by the time we are in that mode our setting should be temporary too, and our temporary setting is development:true our permanent setting would be development:false and not production:true.

The above sounds more logical to me :)

EDIT: development:false would be the default value so if we are ready for production we just remove development:true property from our settings. :)

Cowboy-coder commented 12 years ago

Yeah, totally agree with you there! Will work on all these changes discussed in a couple of days. Thx for the feedback!