Split grunt copy task into copy and uglify to minify JS files

[humphd]

Previously, the grunt build was just copying things from the source tree, which left a lot of JS unminified (i.e., anything in an extension or not in the requirejs build step). This splits the copy process across two tasks, one of which copies, then the second uglfiies things in dist.

I had to do one hack I don't love, but I'm not sure how to fix it. The code for the QuickView extension's main.js breaks when you minify it. I could try and track that down, but it won't save enough space to worry about, so I just skip it (i.e., copy but don't minify).

[gideonthomas]

r=me

[humphd]

Before I land this on our branch, @redmunds would you guys take this patch upstream?

[sedge]

I won't pretend to understand it all, but it looks good.

[redmunds]

@humphd Thanks for asking. Seems like more granularity would be nice to have.

@jasonsanjose @peterflynn @nethip Any reason we wouldn't want this in Brackets?

[peterflynn]

@humphd Is this more aggressive than the Uglify2 task that Brackets currently runs on its main source in the requirejs task? Currently the source is basically just concatenated, not compacted/minified at all. Anything that involves renaming vars would break extensibility etc., and even stripping comments would hurt field-debuggability.

It's worrisome that QuickView gets broken when subjected to this... How sure are you that other features don't have subtler problems? I think I'd want to get to the bottom of what's going wrong with QuickView first, to feel confident it wouldn't cause more widespread breakage.

[humphd]

This is separate from the requirejs build step, since a bunch of thirdparty stuff (CodeMirror, extensions, require.js itself, ...) get copied over to dist/ without any processing, and end up being really a lot bigger than they need to be. This helps with that. NOTE: I've turned off name mangling here, so it's not that.

I share your concerns are other subtle issues, which is why I didn't bother doing this PR against upstream. I can try playing with uglify's options a bit more, and see if I can fix the QuickView code. Before I bothered, I wanted to see if you'd take it at all.

[humphd]

I did some digging, and it's function hoisting that's causing QuickView to break. This fixes things:

        uglify: {
            options: {
                mangle: false,
                compress: {
                    hoist_funs: false

[humphd]

This also exposed an issue in xorigin.js, which I've done a PR for upstream adobe#10683

[peterflynn]

Are there other possible transformations Uglify is doing that could break other parts of Brackets? Is there anything that enumerates all the settings that are on by default?

It seems safest to run Uglify in the same bare-minimum mode (concat only) that Brackets applies to the non-thirdparty stuff... but does that make it not a win for you guys? I.e. is stripping comments, whitespace, etc. important for page load or is gzipping the concat-only result good enough?

[humphd]

The list of compressor options, with info about defaults is documented here https://github.com/mishoo/UglifyJS2#compressor-options.

I did a test, and by simply copying (what happens now for dist/) vs. doing the minification step, I save 136K of gzipped content (from 1074K down to 938K). It's not massive, but it's a help for the browser case.

I think that it's unlikely to be beneficial for the Brackets-proper case, so I've altered my patch so that we can use a build-browser task, which does build + uglification on dist/. This works nicely, except for the issue in adobe#10683. I'm happy to just land this in our branch if it's easier?

[sedge]

r+

[humphd]

OK, I've turned off uglify's name mangling and compression--it's just doing whitespace packing and concatenation now. This is still good.

In later bugs it would be good to figure out how much of uglify's compression stuff we can safely turn on, but that can happen over time.

I've also reworked things so that grunt build does what happens in Brackets upstream, and grunt build-browser does this extra uglification step.

[nethip]

@redmunds Yes! Fine granularity is definently good. But I doubt the uglify build step. Barring this step other steps seems OK with me.

We are also tracking another PR adobe#10219 which will further compress the dist folder.