search

pthrasher / snockets

Sprockets-style script concatenation for Node
119 stars 39 forks source link

SourceMap support / CoffeeScript upgrade / UglifyJS upgrade #42

Closed pthrasher closed 11 years ago

pthrasher commented 11 years ago

First and foremost, this is a pretty big PR. I get that, and I get that large PR's with big changes are generally not something looked kindly upon.

I needed the following changes for my own projects, and once I put them in place, I thought I'd offer you the opportunity to merge them in if you wanted, instead of further fragmenting the "sprockets-like" packages on npm.

SourceMaps are now supported, and they all map back to the original source files, not to a concatenated file. Additionally, if you compiled a coffeescript file, the sourcemap points to the coffeescript, not the compiled javascript -- even when you minify the resulting javascript.

In order to acheive this, I had to refactor the getConcatenation code, update uglifyjs to version 2 (to get source map support) and update coffee-script to latest to get sourcemap support there as well.

I added a unit test to ensure sourcemaps are generated properly, but there could probably be a few more unit tests added around minify and the coffee compile function. Both of which have quite a bit more logic in then than they did before.

All the tests pass, so original expected behavior as far as test coverage remains unchanged.

Again, I understand this is a very large PR, and my feelings certainly won't be hurt if you decide you don't want to pull this in at all. However, I'd prefer to not have to maintain my own fork on github and npm.

TrevorBurnham commented 11 years ago

@pthrasher This is awesome. Do you want to take over as the maintainer of Snockets?

pthrasher commented 11 years ago

@TrevorBurnham Thanks!

Maintaining popular open source projects is a tall order, but I'm definitely willing to do so. My colleagues can pitch in as well, as we'll be using this where I work.

TrevorBurnham commented 11 years ago

@pthrasher Cool. You'll need to move pthrasher/snockets elsewhere so that I can transfer ownership of this repo to you.

pthrasher commented 11 years ago

@TrevorBurnham Done.

TrevorBurnham commented 11 years ago

@pthrasher Hmm, GitHub still won't transfer the repo. It says you still have a repo "in the TrevorBurnham/snockets network." You might need to transfer it to another user or take it down entirely.

pthrasher commented 11 years ago

Ahh, makes sense. Deleted.

TrevorBurnham commented 11 years ago

@pthrasher Alright, transfer request sent. I'll grant you npm publishing packages later today. Great power, great responsibility, etc.!

pthrasher commented 11 years ago

@TrevorBurnham Thank you sir, understood. No intentions of breaking compatibility, etc. Definite intentions of increasing test coverage. I'll add you as a contributor to the project too if you still want access.