search

code4sac / wicit

A simple node/express app for finding locations that accept WIC in California, using data from the new California Department of Public Health open data portal.
http://findwic.com/
MIT License
19 stars 20 forks source link

Concatenate and minify JS #31

Closed donmccurdy closed 9 years ago

donmccurdy commented 9 years ago

This fixes #21 with a separate grunt prod task: Otherwise you need sourcemaps, and ngAnnotate (which makes Uglify play nice with Angular's dependency injection) doesn't support many-to-one concatenation with sourcemaps. Didn't commit the minified files since it's probably best to just run that when deploying.

The other option would be to minify but not concatenate, or just manually write the annotations with controller declarations — that's a little verbose, but would avoid the need for ngAnnotate entirely.

I'm also assuming that process.env.NODE_ENV === 'production' on your Digital Ocean droplet, otherwise it will still serve the unminified files. ;)

Cheers!

jesserosato commented 9 years ago

@donmccurdy :+1: I definitely should have mentioned the issue with Angular's DI, good catch. Good call on the separate task, I prefer using uncompressed scripts in development over source maps anyways (especially on a project that we hope less-experienced devs can work on). It looks like the Angular docs recommend ngAnnotate, but I'm ok with it as is. Will merge as soon as I get a chance.

donmccurdy commented 9 years ago

Sounds good. I did end up using ngAnnotate actually (first paragraph of my comment is badly phrased looking back haha) but only for the grunt prod task, so that should work well enough.

EDIT: Just updated the PR to keep minified CSS out of source as well.

jesserosato commented 9 years ago

@donmccurdy Sorry this took me sooooo long to merge in. Thank you for your contributions, I'll be merging in your remaining open PR ASAP, and I hope you'll think about contributing again if we've got any open issues that interest you, or if you have any ideas for the project. I promise to get to any future PRs much quicker than I got to this one (which is an admittedly low bar). Sorry again for the delay merging this in.

donmccurdy commented 9 years ago

No worries, thanks for the merge! I moved in April/May so I've been keeping busy anyhow.