Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sourcemap is not enabled in development? #139

Closed
kasperpeulen opened this issue Jul 23, 2016 · 28 comments
Closed

Sourcemap is not enabled in development? #139

kasperpeulen opened this issue Jul 23, 2016 · 28 comments

Comments

@kasperpeulen
Copy link
Contributor

It seems like sourcemap is not enabled in development, is there some reason for that?

My normal workflow, is that if I see an error in the chrome console, I look up the file and line number of the error in my editor. But at this moment, the line numbers are not correct, because it doesn't use source mapping.

This leads to another question, why is ES6 transpiled to ES5 in development? Shouldn't it transpile to ES6 now that chrome, edge, safari and firefox all support more ES6 than babel itself support?

I think ES6 to ES5 transpiling is something only needed for production.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 23, 2016

It seems like sourcemap is not enabled in development, is there some reason for that?

I don't think there is one except it was an oversight, that should definitely be added!

This leads to another question, why is ES6 transpiled to ES5 in development? Shouldn't it transpile to ES6 now that chrome, edge, safari and firefox all support more ES6 than babel itself support?

I've never heard of this being done before to be honest, but I think we want to support everybody with create-react-app, even developers that still use IE, so I don't think that'll change!

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

We want to support any browsers currently supported by React (that includes >= IE 9).
As for sourcemaps, unfortunately there is a reason: #109 (comment).

I wonder if this could be fixed on Webpack (or Chrome?) side in any way.
cc @TheLarkInn

@ChrisCinelli
Copy link

What about using cheap-module-eval-source-map?

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

Same problem as I described in #109 (comment).

@taion
Copy link

taion commented Jul 25, 2016

This was a (bizarre) bug fixed in Chrome 53.

We handle this with

config = merge(config, {
  devtool: 'cheap-module-source-map',

  // TODO: Workaround for Chrome < 53. Remove once Chrome 53 hits stable.
  output: {
    devtoolModuleFilenameTemplate: '[absolute-resource-path]',
  },
});

per something cited on some webpack issue somewhere.

eval sourcemaps don't work with breakpoints on startup in general. The "inline" sourcemap would work too, and would obviate the need for the dumb devtoolModuleFilenameTemplate thing, but it imposes a significant penalty on page load time for large apps (from 6s to 30s in our case, which was unacceptable even for development)

@taion
Copy link

taion commented Jul 25, 2016

IMO the best balanced approach is to use cheap-module-source-map as above, stick in an override for devtoolModuleFilenameTemplate, and remove it once Chrome 53 hits the stable channel.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

This is super interesting, thank you for the info. We'll give it a try.

@gaearon gaearon added this to the 0.2.0 milestone Jul 25, 2016
@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

I looked it again. cheap-module-source-map works great with breakpoints, thanks for the tip.

There is however another (IMO rather big) usability problem: https://bugs.chromium.org/p/chromium/issues/detail?id=327092

I understand this is not Webpack’s issue, but not being able to inspect or reference some variables / import in console is very frustrating and completely non-obvious to the beginners.

Canary shipped support for variable mappings but I don’t think Webpack or Babel support this sourcemap spec (I’m not even sure which spec Chrome is supporting.)

So I’m not comfortable enabling sourcemaps in development until Babel and Webpack output them in a way that at least Canary can map the variable names correctly.

@gaearon gaearon removed this from the 0.2.0 milestone Jul 25, 2016
@taion
Copy link

taion commented Jul 25, 2016

I don't think the tradeoff is clearly in favor of "avoid unexpected variable names", BTW – the flip side is that the code you see in the debugger is not the code that you wrote, which also is confusing and strange.

As a caveat, I think "variable bindings like this and imports are a bit messed up" is less bad than "the code you see in the debugger is not the code you wrote, and the variable bindings are still messed up but transparently so" – especially because as transpiled, semantically imports cannot be represented as "normal" variables anyway.

@taion
Copy link

taion commented Jul 25, 2016

Like specifically, at least as long as imports are transpiled with Babel and are not actually supported, something like:

import Foo from './Foo';

needs to be something like FooModule.default rather than just Foo, and it's not just a matter of mapping the variable name.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Fair point about imports.

In general though, I think it’s valuable to understand what the code looks like under the hood. When you see errors, you will see errors about the transpiled code anyway. It is useful to be able to look at the exact code that errored rather than a feel-good ES6 source. (And sometimes even Babel transforms are buggy.)

There are other pain points, like when you click on the breakpoint pane but the breakpoint “falls through” many lines down. This is confusing.

Still, I understand there are many ways to look at this. I’m not saying my opinion is final, but this is the decision I’m taking based on the issues I’ve seen so far. And Babel output can be improved when there is a desire for it.

If there is enough feedback that this was a bad decision, we might change this to what you suggest. For now I’d propose to wait and see.

@taion
Copy link

taion commented Jul 25, 2016

That makes sense. Thanks.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Closing as we can’t / won’t fix this now due to tradeoffs above. We might revisit this in the future.

@taion
Copy link

taion commented Aug 4, 2016

In case anybody is following this, the proper fix for sourcemaps landed in Chrome 52 (current stable), not 53. If you're tracking this to see how to get your source maps working, you can drop that workaround.

@andreypopp
Copy link
Contributor

I understand this project follows 0config philosophy but maybe enabling source maps (via cheap-module-eval-source) could be a configurable option? Seeing incorrect line numbers in stack traces breaks workflow too much for me.

@gaearon
Copy link
Contributor

gaearon commented Sep 8, 2016

Can you file a separate issue about this? Maybe we could try preserving the line numbers (I think Babel could do that). Or just reconsider and make a list of actionable issues in this or other products (Chrome, Webpack) that need to be solved for sourcemaps to become a better default.

@andreypopp
Copy link
Contributor

@gaearon done: #605

@ringzhz
Copy link

ringzhz commented Sep 23, 2016

I'm sorry, but I can't seem to find any answer. Is there a way to enable source mapping with this project? How? Maybe it would be nice to add to the docs.

@gaearon
Copy link
Contributor

gaearon commented Sep 23, 2016

Not currently. I’m reopening because it seems like a lot of people want sourcemaps despite their shortcomings. So we’ll probably add them before 1.0.

@gaearon gaearon reopened this Sep 23, 2016
@gaearon gaearon removed the wontfix label Sep 23, 2016
@gaearon gaearon added this to the 1.0.0 milestone Sep 23, 2016
@arteezy
Copy link

arteezy commented Oct 23, 2016

@taion is there a way to use your workaround, without ejecting from CRA?

@taion
Copy link

taion commented Oct 23, 2016

Chrome 52 has been stable for ages now. That workaround is no longer relevant.

@arteezy
Copy link

arteezy commented Oct 24, 2016

@taion no, I mean setting webpack config, as you mentioned before:

config = merge(config, {
  devtool: 'cheap-module-source-map'
});

Not possible to enable sourcemaps without ejecting?

@axbarchuk
Copy link

@arteezy You can try to do next hook:
Go to node_modules/react-scripts/config/webpack.config.dev.js and change devtools to devtool: 'cheap-module-source-map' it's will be work while you don't remove node_modules folder.

@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

Since people seem to prefer it I merged #924.
It will enable sourcemaps and will be out in 0.8.0.

@gaearon gaearon closed this as completed Nov 20, 2016
@gaearon gaearon modified the milestones: 0.8.0, 1.0.0 Nov 20, 2016
@quide
Copy link

quide commented Nov 30, 2016

Sorry, newby question:
Can I activate source-map without ejecting?

Thanks in advance.
(Debugging React made by create-react-app is not a very easy-searchable topic at Google...)

@gaearon
Copy link
Contributor

gaearon commented Nov 30, 2016

Hi! As I noted in the comment right above:

It will enable sourcemaps and will be out in 0.8.0.

0.8.0 is not out yet so no. When it is out, yes, this will be enabled and you wouldn't need to eject.

@crowdwave
Copy link

crowdwave commented Dec 31, 2016

It would be good to enable source maps via an environment =variable rather than breaking zero configuration.

for source maps:
i.e. cross-env NODE_ENV=development

for no source maps:
either nothing or
i.e. cross-env NODE_ENV=production

fiddling with configuration files is a very short road to JavaScript hell. The least worst option after that is environment variables that the knowledgeable can choose to set if they want, with zero impact on zero config.

@gaearon
Copy link
Contributor

gaearon commented Dec 31, 2016

That’s often what we do, but in this case we just enabled them for everybody since most issues with sourcemaps were resolved.

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests