-
Notifications
You must be signed in to change notification settings - Fork 27.5k
feat(Grunt): add source maps to all min files #2907
Conversation
sourceMap: function(mapFile, fileContents) { | ||
// the // prefix in the string is a workaround for IE's conditional compilation | ||
// see http://bugs.jquery.com/ticket/13274#comment:6 | ||
var sourceMapLine = '//@ sourceMappingURL=' + mapFile + '\n'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue jQuery came across was the use of //@
, which is required for source maps, being confused as //@cc_on
and sometimes throwing in IE. The solution was to wrap the source map pragma comment in a multiline comment block e.g.
/*
//@ sourceMappingURL=...
*/
That's why I added it to the *.prefix
files. But it can easily just be appended to the bottom like you've done here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if you put this line in the non-minified file just like you originally did via .prefix files then browser thinks that it needs to map the non-minified file.
on the other hand if you just inject this line into the minified file after minification and map generation then the mapping line offsets are wrong and stepping through the code is broken. (try debugging with the build of angular from your branch and you'll see that stepping through the code will result in weirdness).
I guess the solution then is to append
/*
//@ sourceMappingURL=...
*/
instead of just
//@ sourceMappingURL=...
to the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I see why you just appended it at the end so the optimal solution would be to just update the sourceMap
method to append the multi-line source map comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution is to switch to the updated standard. :) The pragma has been changed to //# sourceMappingURL
recently. Chrome & WebKit already support the new standard:
https://code.google.com/p/chromium/issues/detail?id=239660
Firefox Nightly is going to do it very soon:
https://bugzilla.mozilla.org/show_bug.cgi?id=870361
Also on the other reason I had the version number in the source map file name was because jQuery team hit an issue that I may have got confused over and it should be fine to keep it clean. |
One more thing, it seems most of the dev tools have added[1] or will add very soon the newer Not sure if it's worth going straight with [1] https://groups.google.com/d/msg/mozilla.dev.js-sourcemap/4uo7Z5nTfUY/_YNQtSxdclkJ |
Generate source map files when build step is ran and adds source map headers to all min files. Source map url must be appended to the min file otherwise the line offsets will be off. Inspired by Ryan Seddon (PR angular#2858) Closes angular#1714
Generate source map files when build step is ran and adds source map
headers to all min files.
Source map url must be appended to the min file otherwise the line
offsets will be off.
Inspired by Ryan Seddon (PR #2858)
Closes #1714