Enable Minification

[MatthewVita]

I'm going to be sending a PR for this (hopefully) by the end of the week :santa:

[DavidSouther]

The biggest issue I have run into is getting Uglify to handle sourcemaps sanely. This will be "complete" when the sourcemap returned with the minified file correctly maps to the original script files (in Chrome).

[MatthewVita]

Thanks for the heads up.

[MatthewVita]

Okay, sorry I'm starting on this so late >.<

If I am understanding the code correctly, https://github.com/RupertJS/stassets/blob/master/lib/Watchers/Script.coffee#L54-#L60 isn't being called at the moment. If that is the case, what's the best way to invoke it?

Also, are we only concerned with JavaScript sourcemaps or do we want coffee as well?

[DavidSouther]

https://github.com/RupertJS/stassets/blob/master/lib/Watchers/Script.coffee#L64

Stassets Watchers are map/reduce, in this case we call them render/concat. Each file in the list of patterns (the thing you create when you specify script: types: [...] gets passed individually through the render function, then the array of all the render results get passed to the concat function. The render function returns an object {content: string, sourceMap: object, path: string} where content is the rendered content for that file type (eg js -> js w/ immediately invoked function, coffee -> js, es6 -> js with System module), sourceMap is an object representation of the source map for that file (eg has field version with value 3, has field mappings with array of mappings, etc), and path is just the string path as a convenience / nonce value. Finally, the concat method returns {content: string, sourceMap: object} where content is the single resulting file content, and sourceMap is a sourceMap object (not string).

minify will get called if {scripts: {compress: true}} is set, the same place you set the types array. The base approach to concatenating files with sourcemap is to do something like compiledFiles.pluck('content').join('\n'), while simultaneously adding each sourceMap to a master sourceMap using the sourceMap Sections mechanism (see "Index map: supporting post processing" in https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.535es3xeprgt

The issue I had with my first attempt at uglify was with trying to use an in-memory sourceMap; uglify wanted the sourceMap to be on disk and referenced using the sourceMap comment in the generated file.

[MatthewVita]

David,

Thanks for that wealth of information, it should help me get started.

I do have a few questions/things for us to consider, but I'll hold off until I can get this thing running (even if "running" at the moment means "half working").

Here's my Script.coffee method:

minify: ({content, sourceMap})->
    console.log 'here!!!'
    content = ngmin.annotate content
    result = UglifyJS.minify content,
        fromString: true
        inSourceMap: sourceMap
        outSourceMap: "app.js.map"
    {content: result.code, sourceMap: result.map}

Here's my server.json:

{
  "stassets": {
    "verbose": true,
    "root": "./src/client",
    "vendors": {
      "prefix": [],
      "js": [],
      "css": [],
      "scripts": {
        "compress": true
      }
    }
  },
  "tls": false,
  "routing": [
    "./src/server/allow.coffee",
    "./src/server/**/route.coffee"
  ]
}

(This is rupert w/ Angular, BTW.)

It doesn't minify and it doesn't say 'here' in stdout :(

[DavidSouther]

scripts goes under stassets, not under vendors. I'll update the docs.

[MatthewVita]

ughh, I knew that... my bad!!!

[MatthewVita]

dang, still not compressing, generating a sourcemap or writing a message to stdout. I'll keep debugging and will update you

[DavidSouther]

You know, I might have missed this but why are you using a rupert config object? You should be testing this using ./test/server/app.js and add the compress value in ./test/server/config.js

[MatthewVita]

https://gist.github.com/MatthewVita/dba72f48970fa391b457

[MatthewVita]

Looks like I'm going to give this a try https://github.com/mishoo/UglifyJS2#the-hard-way ...did you try that path ever?

[DavidSouther]

Looks like it's using Mozilla's source maps under the hood, which don't support sections

[MatthewVita]

how do you feel about trying out the closure compiler for this? Here's a JRE-free one: https://github.com/dcodeIO/ClosureCompiler.js

[DavidSoutherTravis]

Bounced the idea around a few times. Go for it. Using the jar is fine, I can point out dibs techniques projects use to keep the local jar up to date.

On Wed, Jan 7, 2015, 22:22 Matthew Vita notifications@github.com wrote:

how do you feel about trying out the closure compiler for this? Here's a JRE-free one: https://github.com/dcodeIO/ClosureCompiler.js

— Reply to this email directly or view it on GitHub https://github.com/RupertJS/stassets/issues/23#issuecomment-69130843.

[MatthewVita]

We can switch to the jar once we're sure this is the direction to go, no?

Here's a minify implementation with the CC. It seems that FILES are the only way for this to work, not in memory values... do you know any way around this? Have you experienced this in the past?

minify: ({content, sourceMap}) ->
    unix = unixTime.now()
    tmpFiles =
        minFile: unix + '.min.js'
        mapFile: unix + '.map.js'
        minData: ''
        mapData: ''

    try
        fs.writeFileSync tmpFiles.minFile, content, 'utf-8'
    catch
        return err

    ClosureCompiler.compile tmpFiles.minFile,
        compilation_level: "ADVANCED_OPTIMIZATIONS",
        create_source_map: tmpFiles.mapFile,
    , (err, result) ->
        if err
            return err

        tmpFiles.minData = result

        try
            tmpFiles.mapData = fs.readFileSync(tmpFiles.mapFile).toString()
        catch err
            return err

        [tmpFiles.minFile, tmpFiles.mapFile].forEach (file) ->
            try
                fs.unlinkSync(file)
            catch err
                return err

        return {content: tmpFiles.minData, sourceMap: tmpFiles.mapData}

...which results in

{
    content: 'angular.a("stassets.main",["stassets.main.controller","main.template"]);angular.a("stassets.main.nav.service",[]).e("NavSvc",function(){});(function(){var a;a=function(){return function(){}}();angular.a("stassets.main.controller",[]).b("MainCtrl",a)}).call(this);angular.a("stassets.main.nav.directive",["main.nav.template","main.nav.service"]).c("stassetNav",function(){return{d:"AE",f:"main/nav"}});\n',

    sourceMap: '{\n"version":3,\n"file":"",\n"lineCount":1,\n"mappings":"AACEA,OAAAC,EAAA,CAAe,eAAf,CAAgC,CAAC,0BAAD,CAA6B,eAA7B,CAAhC,CAMFD,QAAAC,EAAA,CAAe,2BAAf,CAA4C,EAA5C,CAAAC,EAAA,CACW,QADX,CACqBC,QAAe,EAAE,EADtC,CAOC,UAAQ,EAAG,CACV,IAAIC,CAEJA,EAAA,CAAY,QAAQ,EAAG,CAGrB,MAFAA,SAAiB,EAAG,EADC,CAAZ,EAOXJ,QAAAC,EAAA,CAAe,0BAAf,CAA2C,EAA3C,CAAAI,EAAA,CAA0D,UAA1D,CAAsED,CAAtE,CAVU,CAAX,CAADE,KAAA,CAYQ,IAZR,CAeEN,QAAAC,EAAA,CAAe,6BAAf,CAA8C,CAAC,mBAAD,CAAsB,kBAAtB,CAA9C,CAAAM,EAAA,CAAmG,YAAnG,CAAiH,QAAQ,EAAG,CAC1H,MAAO,CACLC,EAAU,IADL,CAELC,EAAa,UAFR,CADmH,CAA5H;",\n"sources":["1420772603.029.min.js"],\n"names":["angular","module","service","NavSvc","MainCtrl","controller","call","directive","restrict","templateUrl"]\n}\n'
}

[MatthewVita]

btw, I'm not a huge fan of this implementation as it's not very elegant and have a feeling I overlooked some things

[DavidSouther]

Yeah that's a total no-go.

First, the content output is clearly incorrect and asking end users to take the time to be ADVANCED_OPTIMIZATIONS compatible is a non-starter. (Take that back - use config.find('scripts.optimization', 'OPTIMIZATION_LEVEL', 'SIMPLE_OPTIMIZATIONS') for the compilation_level). Second you'll need to require rupert-plugin-s to include export definitions for Closure. So basically take everything I'm mentioning here, and put it rupert-plugin-closure-compiler with appropriate options, etc.

It looks like we're going to need to stick with uglify for the base functionality.

[MatthewVita]

Right.

Regardless, it doesn't seem like we'll be able to get around using files with closure compiler. I've concluded this with the following experiment (using the raw jar file instead of a wrapper lib):

test.js is just console.log("test");

$ java -jar compiler.jar --js_output_file=output.js test.js
$ cat output.js 
console.log("test");
$ java -jar compiler.jar --js_output_file=output.js 'console.log("test");'
ERROR - Cannot read: console.log("test");

1 error(s), 0 warning(s)

So 2 questions for you: 1) Do you know of anyway to use CC with in memory values? (Kind of like the fromString: true options in Uglify) 2) Why is there a sourceMap parameter for the minify method? Wouldn't this be a result of minify, rather than an input?

[DavidSouther]

Ok.

1) Temp files are totally OK - we're making one (application.js) (vendors.js should already be minified individually, so the concat overhead is minimal). To make it, use https://github.com/bruce/node-temp

2) The concatenation happens in SourceMapWatcher::concat (63: res = super _ # {content, sourceMap}). At this point, content is the content of each file joined with '\n' and sourceMaps is the one source map (see https://github.com/RupertJS/stassets/issues/23#issuecomment-68961310, second paragraph). That pair of complete files is passed to minify. Hmm, I just explained it using the exact same words... if that's a problem, I'll try again.

[MatthewVita]

Okay, I'll use temp files with CC and get back to you (obviously going to include the sourceMap in parameter this time around). Thanks for the advice with node-temp... very convenient.

As for the sourceMap, I believe I'm clear on it now... my confusion probably stemmed from not considering that coffeescript/es6 may need to pass in a sourcemap where plain JS does not (if I understand correctly).

[DavidSouther]

my confusion probably stemmed from not considering that coffeescript/es6 may need to pass in a sourcemap where plain JS does not

You're correct here, but in fact JS does also pass along a source map for consistency (though it's a 1:1 map :) )

[MatthewVita]

interesting, thanks for the clarification!

[MatthewVita]

Closure Compiler does not support IN sourcemaps, so that's a no go.

Here's a minify using in memory values that appears to look right (in terms of the minified code + resulting sourcemap) but the sourcemapping doesn't seem to work in the browser... can you give it a try? This is just something I wanted to put together in the interim really:

minify: ({content, sourceMap})->
    deferred = Q.defer()

    result = UglifyJS.minify content,
        fromString: true
        outSourceMap: "app.js.map"

    if typeof result.code is "undefined" or typeof result.map is "undefined"
        deferred.reject new Error "can't minify"
    else
        deferred.resolve {content: result.code, sourceMap: result.map}

    return deferred.promise

In terms of the "actual solution"... I know you posted this https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.535es3xeprgt but I'm not sure what the right format is for this "master source map" file.

I'm thinking of using the temp library that you recommended for appending together a big source map via the values in:

sourceMap.sections.forEach (v, k) ->
    console.log v.map

...but, again, I'm not sure what the "rules" are for this master sourcemap file and if it would even work?

[DavidSouther]

I can explain it... but I'm going to need a whiteboard. Are you around for coffee sometime this weekend?

[MatthewVita]

unfortunately my weekend is booked... what's your week after 3:30pm looking like? Other than that google doc you linked me, are there any other resource I can check out regarding this "master source map" file?

As for the new minify function I posted... do you think we can implement something like that in the interim? (sourcemaps only to the final javascript)

[DavidSouther]

I'm pretty open at this point, though my evenings are filled with meetups.

I don't see the need for an interim solution at this point. It's a good stepping stone, but no need to push aggressively on it with no need for it (today).

[MatthewVita]

2015-01-15 16:00:00 ? where do you want to meet if that works

[DavidSouther]

Sure. How about CoffeeTree in Bakery Square?

[MatthewVita]

Okay. See you Thursday at 4pm at Coffeetree in Bakery Square.

[MatthewVita]

Can you publish my recent commits to https://github.com/DavidSouther/flatten-source-map to NPM?

[MatthewVita]

btw, this was what I was thinking for minify. Thoughts?

minify: ({content, sourceMap})->
    deferred = Q.defer()

    generatedMap = flatten(sourceMap);

    if typeof generatedMap.sourcesContent is "undefined" or typeof generatedMap.mappings is "undefined"
        deferred.reject new Error "Can't minify"
    else
        deferred.resolve {content: generatedMap.sourcesContent.join(''), sourceMap: generatedMap.mappings}

    return deferred.promise

[MatthewVita]

bump :+1:

what do you think of the approach that I recently listed? do we need a content param afterall?

[DavidSouther]

Sorry, must have missed this. No, it is not appropriate to fail the minification if the mappings or sourcesContent are undefined.

[MatthewVita]

hmm, so should I just do a resolve with nothing in that case?

[DavidSouther]

The content value passed in to minify is a valid JS block. Anything passed to the resolve should have that content - either as is, or minified. If the source map fails to flatten, that is an issue - but should not result in the javascript itself becoming incorrect. flatten-sourcemap only prepares the source map to be used with Uglify.

[MatthewVita]

idk what I was thinking... I failed to include in the previous Uglify code I posted with this flatten sourcemap example.

I suppose using content as a param for Uglify is a valid case, but we can also just strip out the content from the sourceMap in a loop. The former is faster though.

Let me get you an actual example of this...

[MatthewVita]

... https://github.com/RupertJS/stassets/pull/34 thoughts? It seems to work from my testing