-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable Minification #23
Comments
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). |
Thanks for the heads up. |
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? |
https://github.com/RupertJS/stassets/blob/master/lib/Watchers/Script.coffee#L64 Stassets Watchers are map/reduce, in this case we call them
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. |
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 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
(This is rupert w/ Angular, BTW.) It doesn't minify and it doesn't say 'here' in stdout :( |
|
ughh, I knew that... my bad!!! |
dang, still not compressing, generating a sourcemap or writing a message to stdout. I'll keep debugging and will update you |
You know, I might have missed this but why are you using a rupert config object? You should be testing this using |
Looks like I'm going to give this a try https://github.com/mishoo/UglifyJS2#the-hard-way ...did you try that path ever? |
Looks like it's using Mozilla's source maps under the hood, which don't support sections |
how do you feel about trying out the closure compiler for this? Here's a JRE-free one: https://github.com/dcodeIO/ClosureCompiler.js |
Bounced the idea around a few times. Go for it. Using the jar is fine, I On Wed, Jan 7, 2015, 22:22 Matthew Vita notifications@github.com wrote:
|
We can switch to the jar once we're sure this is the direction to go, no? Here's a 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
|
btw, I'm not a huge fan of this implementation as it's not very elegant and have a feeling I overlooked some things |
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 It looks like we're going to need to stick with uglify for the base functionality. |
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 $ 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:
|
Ok.
|
Okay, I'll use temp files with CC and get back to you (obviously going to include the sourceMap As for the |
You're correct here, but in fact JS does also pass along a source map for consistency (though it's a 1:1 map :) ) |
interesting, thanks for the clarification! |
Closure Compiler does not support IN sourcemaps, so that's a no go. Here's a 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 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? |
I can explain it... but I'm going to need a whiteboard. Are you around for coffee sometime this weekend? |
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) |
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). |
2015-01-15 16:00:00 ? where do you want to meet if that works |
Sure. How about CoffeeTree in Bakery Square? |
Okay. See you Thursday at 4pm at Coffeetree in Bakery Square. |
Can you publish my recent commits to https://github.com/DavidSouther/flatten-source-map to NPM? |
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 |
bump 👍 what do you think of the approach that I recently listed? do we need a |
Sorry, must have missed this. No, it is not appropriate to fail the minification if the mappings or sourcesContent are undefined. |
hmm, so should I just do a resolve with nothing in that case? |
The |
idk what I was thinking... I failed to include in the previous Uglify code I posted with this flatten sourcemap example. I suppose using Let me get you an actual example of this... |
... #34 thoughts? It seems to work from my testing |
I'm going to be sending a PR for this (hopefully) by the end of the week 🎅
The text was updated successfully, but these errors were encountered: