#13274 closed bug (fixed)
IE < 9: "'jquery' is undefined" w/ sourceMappingURL comment
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | blocker | Milestone: | 1.9.1 |
Component: | build | Version: | 1.9.0 |
Keywords: | Cc: | paul.irish | |
Blocked by: | Blocking: |
Description
When using MSIE < 9 and jQuery 1.9.0 I am getting the following warning/error:
'jquery' is undefined
This error is coming from line 4 in the minified versions, which is (a comment!):
//@ sourceMappingURL=jquery.min.map
Change History (25)
comment:1 Changed 11 years ago by
Owner: | set to alexc223@… |
---|---|
Status: | new → pending |
comment:2 Changed 11 years ago by
I can confirm this bug. You can see it live on http://www.achievementstats.com/
comment:3 Changed 11 years ago by
Component: | unfiled → build |
---|---|
Milestone: | None → 1.9.1 |
Priority: | undecided → blocker |
Status: | pending → open |
It definitely is happening on those sites, although I can't reproduce it with this simple jsfiddle: http://jsfiddle.net/dmethvin/vvtJN/3/
If you get into the IE8 debugging console and type //@ sourceMappingURL=jquery.min.map
you see the same error as above though, so it does seem real. We just need to figure out what conditions cause it.
comment:4 Changed 11 years ago by
I am starting to feel sick...it's conditional code.
http://www.javascriptkit.com/javatutors/conditionalcompile.shtml
If someone, anyone, uses //@cc_on
before jQuery loads it looks like the sourceMappingURL line will cause an error in IE8.
Supposedly it can be turned off with @cc_off
but that would have to be in OUR source file for defensive purposes and would affect following code.
comment:5 Changed 11 years ago by
This will affect *all versions* of IE, not just IE8. See this fiddle in IE10 or below.
comment:6 follow-up: 20 Changed 11 years ago by
Okay spent some time on this with Jake Archibald...
This is where WebKit parses the comment. It does NOT need to start on a newline, but there are some other specifics in the regex it's looking for: https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/inspector/ContentSearchUtils.cpp&exact_package=chromium&q=sourceMappingURL&type=cs&l=143
Two possible fixes...
jQuery-side fix..
Augment the current reference with flowerbox comments (on new lines).
/* //@ sourceMappingURL=undeclaredIdentifier */
cc_on's will still work (and so will the sourcemapping), but IE won't try to parse this as a CC directive.
User-side fix
Since this is only repro'd when a page turns on conditional compilation before including jQuery, DONT DO THAT.
- Don't use @cc_on
- Include jQuery before one of your scripts turns on @cc_on
- stop using some super old legacy version of the html5shiv and update to the latest version
- Don't use @cc_on. Use this instead https://gist.github.com/527683
- @cc_on dramatically slows down js evaluation. don't use it.
comment:7 Changed 11 years ago by
Yep, I can confirm that's the case - an old html5shiv was in use, which uses conditional compilation. Upgrading to the latest html5shiv (which no longer uses it), has resolved this for me.
comment:8 Changed 11 years ago by
AchStats.com uses this table sort script: http://www.frequency-decoder.com/2006/09/16/unobtrusive-table-sort-script-revisited It has indeed conditional code, but that's nothing we could change. However loading jQuery before loading the sort script has resolved the issue! Thanks for the research. :)
comment:9 Changed 11 years ago by
As it turns out, we can fix this and even save a byte: https://github.com/jquery/jquery/pull/1143
Whether we want to remains an open question.
comment:10 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Fix #13274: Wrap sourceMap directive in multiline comments. Close gh-1143.
Changeset: ac93559eb9f18fcaec95dfdc97358b1b85bfe234
comment:11 Changed 11 years ago by
Fix #13274: Wrap sourceMap directive in multiline comments. Close gh-1143. (cherry picked from commit ac93559eb9f18fcaec95dfdc97358b1b85bfe234)
Changeset: 487b703521e63188102c73e8ce6ce203d28f260b
comment:12 Changed 11 years ago by
I just noticed now in master that the //@ sourceMappingURL=
indicator is now at the top of the document.
According to the version 3 spec for source maps, it explicitly says it should be at the end of the document. I'm not sure how these are implemented at the browser level, but could it cause any issues? Or should this idea be brought up to browser devs and have the spec loosened?
Spec document: https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.txk3vdsh99hf
Compiled source from master: https://gist.github.com/raw/43037ab16b9b22d1ceb0/gistfile1.js
comment:13 Changed 11 years ago by
I observe that particular statement to be in the "Conventions" section, and also that a strict reading would prohibit the trailing */
required by this ticket. Given that our current approach was specifically crafted to match browser implementation, who do we talk to about updating the spec?
comment:14 Changed 11 years ago by
Cc: | paul.irish added |
---|
Probably good to pull in Paul Irish here. I am not at all a fan of the current "convention" of using a newline-terminated comment at the end of the file. It also makes more sense for the comment to be a the top of the file. But we should get all the parties involved to agree on what is within the spec.
While we're changing the spec it would be great to pick a sequence that wasn't already in use by @cc_on.
comment:15 Changed 11 years ago by
According to the source of both WebKit and FF, this will technically work. Both work by just looking for a comment that fits that pattern, which technically could be left open to the sourceMappingURL indicator being *anywhere* in the document.
(Slightly out of the scope of this discussion), but I think the beginning of the document makes a lot of sense for implementations other than a browser for one good reason:
It's possible to stream in bytes and detect a source map without loading the whole file into memory. If the source map could be detected early, you can disregard the rest and just fetch the source.
Granted, it's a small idea, but it makes sense to me. Or just strictly use the proposed SourceMap header, which seems the cleanest of them all.
comment:16 Changed 11 years ago by
Using the HTTP header may seem like a good idea but isn't. Most web devs can change the content of a file with no problem but can't change the headers sent with the file without access to server configuration. It also won't work if the file is used locally.
comment:17 Changed 11 years ago by
@dmethvin Good point. It is harder to maintain a header. So maybe just a preference for a header if possible. :)
comment:18 Changed 11 years ago by
I still get an error in IE8, even when using the fix:
/* @ sourceMappingURL=undeclaredIdentifier */
comment:19 Changed 11 years ago by
Sorry, here it is in a proper code block:
/* //@ sourceMappingURL=undeclaredIdentifier */
comment:20 Changed 10 years ago by
Paul, do you have any numbers/tests/explanation for why turning on cc slows down js? Is this in IE only (given that other browser should be ignoring it as a comment)?
Replying to paul.irish:
Okay spent some time on this with Jake Archibald...
This is where WebKit parses the comment. It does NOT need to start on a newline, but there are some other specifics in the regex it's looking for: https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/inspector/ContentSearchUtils.cpp&exact_package=chromium&q=sourceMappingURL&type=cs&l=143
Two possible fixes...
jQuery-side fix..
Augment the current reference with flowerbox comments (on new lines).
/* //@ sourceMappingURL=undeclaredIdentifier */cc_on's will still work (and so will the sourcemapping), but IE won't try to parse this as a CC directive.
User-side fix
Since this is only repro'd when a page turns on conditional compilation before including jQuery, DONT DO THAT.
- Don't use @cc_on
- Include jQuery before one of your scripts turns on @cc_on
- stop using some super old legacy version of the html5shiv and update to the latest version
- Don't use @cc_on. Use this instead https://gist.github.com/527683
- @cc_on dramatically slows down js evaluation. don't use it.
comment:21 Changed 10 years ago by
Will this be fixed in a future version? I have web essentials installed which will automatically change my minified files back to the single line comment. The workaround of a multiline flower comment does not work with Web Essentials.
comment:22 follow-up: 23 Changed 10 years ago by
On our end, the only option we have is to remove support for sourcemaps altogether.
Your tools should not be mangling that comment, I can't think of a reason why they would do that. If you are combining/(re)compressing files the comment becomes useless anyway because the map no longer represents the new file. Remove it or remove it not, there is no mangle.
comment:23 Changed 10 years ago by
Replying to dmethvin:
On our end, the only option we have is to remove support for sourcemaps altogether.
Your tools should not be mangling that comment, I can't think of a reason why they would do that. If you are combining/(re)compressing files the comment becomes useless anyway because the map no longer represents the new file. Remove it or remove it not, there is no mangle.
What I'm saying is that Web Essentials automatically creates the minfied file and map file when you save the unminified file. So if I save the unminified file, the minified file gets re-created with the single line comment.
comment:24 Changed 10 years ago by
Well that's not jQuery's issue at all, Web Essentials is creating an incompatible file. You'll need to let the Web Essentials people know they're generating a file that causes errors in IE and/or convince the sourcemap folks to adopt a syntax that is compatible with IE's @cc_on. I haven't had much luck with the latter myself.
comment:25 Changed 10 years ago by
@dmethvin I've talked to Nick Fitzgerald at the Front Trends conference in Warsaw recently and he's open to changing the @ symbol, in his presentation about source maps he even directly said that it'll probably change. I'm optimistic about that.
Can you provide a test page that we can use? I include *just* the minified 1.9.0 and it seems to work fine.