Side navigation
#13776 closed bug (fixed)
Opened April 14, 2013 08:27PM UTC
Closed April 18, 2013 05:36PM UTC
Last modified May 24, 2013 10:01AM UTC
License comment is breaking the SourceMap
Reported by: | mattrobenolt | Owned by: | gibson042 |
---|---|---|---|
Priority: | high | Milestone: | 1.10/2.0 |
Component: | build | Version: | 1.9.1 |
Keywords: | Cc: | paul.irish | |
Blocked by: | Blocking: |
Description
At the moment, the
@sourceMappingURLcomment is getting injected into the top of the file ''after'' the sourcemap has been generated, resulting in everything being skewed by a few lines.
I've built a tool to help detect these issues: http://sourcemap-validator.herokuapp.com/validate?url=http%3A%2F%2Fcode.jquery.com%2Fjquery-1.9.1.min.js
Attachments (0)
Change History (12)
Changed April 14, 2013 08:33PM UTC by comment:1
component: | unfiled → build |
---|---|
milestone: | None → 1.10/2.0 |
priority: | undecided → high |
status: | new → open |
Changed April 14, 2013 08:45PM UTC by comment:2
What's the reason for it being injected at the top? It seems to be just to save a couple bytes, since there's already a block comment? As opposed to having one at the top and one at the bottom?
Changed April 14, 2013 10:49PM UTC by comment:3
See #13274 for info. If someone uses //@sourceMappingURL=...
in their file it won't work in IE if conditional JS is turned on. Your tool will want to check for that.
I'd actually prefer to see a different comment sequence entirely since you can see what it requires for us to fix.
Changed April 14, 2013 10:54PM UTC by comment:4
Yeah, I followed along with that. Unfortunately,
//@ sourceMappingURL=is hardcoded into browsers currently, so something will have to change upstream to change that sequence of characters. If we want to petition for that, I'm open.
The spec is here: https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit
Mailing list: https://lists.mozilla.org/listinfo/dev-js-sourcemap
FWIW, the spec also specifically says at the bottom of the document, but according to both FF and Webkit source, neither care. It can be anywhere in the document.
In my Python implementation, I explicitly check both because of jQuery. :) https://github.com/mattrobenolt/python-sourcemap/blob/master/sourcemap/__init__.py#L28-L35
Changed April 14, 2013 11:01PM UTC by comment:5
Enclosing the
//@ sourceMappingURL=jquery.min.map
comment withing /* */
comment block solves the IE conditional JS compilation, doesn't it? It could be at the bottom of the file then, although it would add a few bytes, obviously.
dmethvin, where would you put this placeholder? Won't uglifyjs source map generator account for it, thus making the workaround futile?
TBH, I'd prefer browser vendors (and the spec) to accept /*@ sourceMappingURL=...*/
, too. Will that fix the IE conditional compilation problem? If so, it's worth to report it on crbug.com, Chrome guys respond quite quickly to DevTools related issues.
Or maybe just the @
symbol should be changed to sth non-conflicting with IE.
Changed April 14, 2013 11:05PM UTC by comment:6
Backbone does what you're suggesting: http://documentcloud.github.io/backbone/backbone-min.js They just do the whole:
/* //@ sourceMappingURL=backbone-min.map */
It feels really lame though. :( This should get pushed up to browsers before this becomes too mainstream and harder to change, in my opinion.
Changed April 14, 2013 11:15PM UTC by comment:7
cc: | → paul.irish |
---|
Changed April 14, 2013 11:16PM UTC by comment:8
I agree spec is not ideal here. Using IE conditional JS switches is a bad idea but unfortunately it happens.
I raised the issue here: https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.js-sourcemap/4uo7Z5nTfUY
Changed April 17, 2013 08:59PM UTC by comment:9
owner: | → gibson042 |
---|---|
status: | open → assigned |
grunt-contrib-uglify
appears to ignore banner
when generating maps, making them invalid for the resulting minified files. I'll try working around it on our end, but will also file a ticket there.
Changed April 18, 2013 05:36PM UTC by comment:10
resolution: | → fixed |
---|---|
status: | assigned → closed |
Fix #13776: Add banner before generating source map. Close gh-1246.
Changeset: 9d16fe6283667396094d49559a37fc672c06252c
Changed April 18, 2013 06:14PM UTC by comment:11
Upstream issue: https://github.com/gruntjs/grunt-contrib-uglify/issues/22
Changed May 24, 2013 10:01AM UTC by comment:12
This has actually been fully fixed with this bug: http://bugs.jquery.com/ticket/13793
Thanks for catching this! An easy solution would be to put a placeholder in the file I suppose.