Skip to content
This repository has been archived by the owner on Mar 5, 2020. It is now read-only.

Add JS and CSS sourcemaps. #147

Merged
merged 2 commits into from
Mar 5, 2015
Merged

Add JS and CSS sourcemaps. #147

merged 2 commits into from
Mar 5, 2015

Conversation

toolness
Copy link
Contributor

@toolness toolness commented Mar 5, 2015

As mentioned in #139 (comment), this adds JS and CSS source maps to all builds, regardless of whether they're dev or prod.

Since the source maps are provided in separate .map. files that only dev tools access, they shouldn't impact the load times of production builds, and they also encourage anyone to delve into our site and see how it works!

@toolness
Copy link
Contributor Author

toolness commented Mar 5, 2015

@alicoding does this look OK to you? Aside from production-related tasks like minification, is there anything #139 does that this doesn't do?

@@ -6,6 +6,7 @@ var index = require('./lib/index-static.jsx');

module.exports = {
entry: './lib/main.jsx',
devtool: 'source-map',
Copy link
Collaborator

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.

Copy link
Contributor Author

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:

2015-03-05_12-11-36

As you can see there it's correctly mapping to the original JSX source.

However, see that console.log("HI") right before the debugger statement? This is what that looks like in the console:

2015-03-05_12-12-29

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. :(

Copy link
Collaborator

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 set eval manually for Firefox or use the debugger.

Copy link
Contributor Author

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 to eval does make console logging work on Firefox!!!!

2015-03-05_12-26-17

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 to bundle.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.

Copy link
Contributor Author

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 with eval.

@alicoding alicoding mentioned this pull request Mar 5, 2015
@alicoding
Copy link
Collaborator

r+ with updating the README regarding Firefox users how can they debug here.

toolness added a commit that referenced this pull request Mar 5, 2015
@toolness toolness merged commit 6bcd719 into mozilla:master Mar 5, 2015
@toolness
Copy link
Contributor Author

toolness commented Mar 5, 2015

Good idea, I've added an optional WEBPACK_DEVTOOL env var in 223d71c and documented it in the readme, so folks can change the setting if they want.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants