This repository has been archived by the owner on Mar 5, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 91
Add JS and CSS sourcemaps. #147
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this doesn't work for Firefox for some reason and I have to set it to
eval
to get it to work.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.
Hmm, what do you mean by "work for Firefox"? In general I've found that Firefox has very poor support for source maps--they work fine in the debugger, but not the console.
For instance, here's what a
debugger
statement does in Firefox, with the code in this PR applied:As you can see there it's correctly mapping to the original JSX source.
However, see that
console.log("HI")
right before thedebugger
statement? This is what that looks like in the console:In other words, it's not source-mapping the location of the logging statement.
This is particularly frustrating for me because I spend much more of my time in the console than I do in the debugger. But I've never found a project that works any differently on Firefox, so when I really need source-mapped
console.log()
statements, I just switch to Chrome. It's kind of Chrome devtools' killer feature for me. :(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.
Exactly the problem I was facing and had to set to
eval
to get this working both on debugger and console, but lets land this one for now and document that in order to get the console to work they either have to seteval
manually for Firefox or use the debugger.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.
Whoa, yeah, you're totally right, setting
devtool
toeval
does make console logging work on Firefox!!!!WOW.
Unfortunately, it's totally dumb that this won't work on production, since it uses
eval()
to evaluate the code and thus won't work w/ minified stuff. It really sucks that Firefox's support for.map.
files is limited to the debugger only.Another thing I've just noticed about the
eval
approach is that it's actually only showing us the JSX file after the JSX transform. Which for Firefox console logging is still way better than referring tobundle.js
, but for all other use cases (e.g. Chrome, the FF debugger) it's worse, since those other use cases show us the original JSX source when we use.map.
files. So there is no clear winner here, which is lame.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.
Hmm, I just noticed that the exception tracebacks shown in mocha browser tests are also un-sourcemapped when using
source-map
, even in Chrome, but they work fine witheval
.