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

sourcemaps and runtime variables misaligned, yielding difficult debugging #1557

Closed
cdaringe opened this issue Feb 14, 2017 · 18 comments
Closed

Comments

@cdaringe
Copy link

Can you reproduce the problem with latest npm?

yes

Can you still reproduce it?

yes

Description

The sourcemaps appear to work, however, the browser runtime (chrome, in my case) does not honor variable names.

screen shot 2017-02-14 at 10 33 46 am

^ Notice how parseUrl is defined and valid, yet I cannot access it in the render scope.

screen shot 2017-02-14 at 10 33 57 am

^ in fact, _url.parse is a thing. i don't understand why url was mangled to _url or why a new var parseUrl was not put into scope.

I realize that this might be a browser issue, vs a create-react-app issue, but can't say for certain because it may related to CRA's source-map generation.

Expected behavior

Have vars accessible in scope as written in source

Actual behavior

vars are mangled and hidden from the user.

Environment

Run these commands in the project folder and fill in their results:

  1. npm ls react-scripts (if you haven’t ejected): 0.9.x
  2. node -v: 7.4.1
  3. npm -v: 4.12.1, yarn 0.19.1

Then, specify:

  1. Operating system: el cap
  2. Browser and version: chrome 56.0.2

Reproducible Demo

@cdaringe cdaringe changed the title sourcemaps and runtime variables misaligned, yielding difficult debuggin sourcemaps and runtime variables misaligned, yielding difficult debugging Feb 14, 2017
@gaearon
Copy link
Contributor

gaearon commented Feb 14, 2017

Yes, unfortunately it's a known issue with sourcemaps in Babel (and everywhere, really), and one of the reasons I argued against including sourcemaps. However, a lot of people find them very valuable so we opted to include them despite the drawbacks.

I think there was some sort of change to sourcemap spec to retain variable mapping information. But I don't know if either Babel, Webpack, or Chrome are taking advantage of it.

Probably relevant issues:

cc @sokra @hzoo who might know more.

@gaearon
Copy link
Contributor

gaearon commented Feb 14, 2017

As for your particular case I'm not even sure if support for mangled mapping will help.

^ in fact, _url.parse is a thing. i don't understand why url was mangled to _url or why a new var parseUrl was not put into scope.

It is necessary because of ES6 spec. In ES6, imports are “live” so the only way to implement it in ES5 is to have a variable indirection for the import itself. It’s the same reason you see abc.default when you import abc from 'abc': Babel needs to put the import behind an object so that it stays “live” per spec.

See also http://www.2ality.com/2015/07/es6-module-exports.html (search for “live” on the page).

@hzoo
Copy link

hzoo commented Feb 14, 2017

I only remember babel/babel#3658 which broke webpack and also referenced before in #1188

@Timer
Copy link
Contributor

Timer commented Feb 14, 2017

As I recall, the patch to webpack (source-list-map) simply disregarded the new mappings.
Maybe we can find someone willing to add support?

@gaearon
Copy link
Contributor

gaearon commented Feb 14, 2017

It would be nice to verify that without Webpack in the middle, sourcemaps produced by Babel give meaningful results in Chrome. Just so we have a confirmation it's worth doing the work on Webpack side.

@Timer
Copy link
Contributor

Timer commented Feb 14, 2017

I think we just need to test with full-blown sourcemaps enabled (during development, I may be wrong tho), because source-list-map is what is used for one-mapping-per-line iirc. webpack might also strip this elsewhere.

@Timer
Copy link
Contributor

Timer commented Feb 15, 2017

@cdaringe do you have time to test if sourcemaps produced by Babel (without webpack) function correctly as @gaearon described above? Help would be greatly appreciated.

@cdaringe
Copy link
Author

cdaringe commented Feb 16, 2017

i made an example here, only to realize that of course pure babel is ok--its babel-loader w/ uglify that warrant these difficult source maps. i also did an example with babili. all is green 🍏, but it's a biased test because babel & babili are maintaining my symbol names.

@cdaringe
Copy link
Author

nm babili is mangling some stuff & its busted per the same failure mode.

pure babili example above:
screen shot 2017-02-15 at 9 22 42 pm

check out that wild local var!

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2017

For now I don't think we care about what happens with Babili as we don't ship with it.
Also I am more interested in mapping working in DEV mode (in which we don't even Uglify).

@cdaringe
Copy link
Author

then the first example should suffice :). however, theres no bundler or uglification/mangling w/ pure babel, so no reason for the failure mode described above to affect it.

@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

What is the next actionable step here? Do we need to file an issue with one of the projects? Which one?

@cdaringe
Copy link
Author

cdaringe commented Feb 26, 2017

sorry, my remarks didn't add clarity. lets fix that! your original thoughts were ~~accurate.

  • babel, without ES2015 module imports, works fine with sourcemaps.
  • babel, with ES2015 module imports, does not work w/ sourcemaps

therefore there should be a followup in babel-land on this. this can be easily demonstrated by cloning & running this tiny example. because we wanted a webpack-free example, i stub in a window.require that simply returns React. the example uses a pure babel build (using CRA's babel config) w/ a fake require. Again, Chrome cannot show in-scope imports, such as Component.

@wasifhyder
Copy link

wasifhyder commented May 15, 2017

I'm able to get the variable bindings aligned by using the following .babelrc file.

// .babelrc
{
  "presets": [
    "react",
    ["es2015", {"modules": false}]
  ],
  "plugins": [
    ["transform-es2015-modules-commonjs-simple", {
      "noMangle": true
    }]
  ],
  "sourceMaps": true
}

@cdaringe
Copy link
Author

cdaringe commented May 15, 2017

@wasifhyder, nice! i haven't tested it, but if you're confident, you should drop a PR down on babel-preset-react-app

@gaearon
Copy link
Contributor

gaearon commented May 15, 2017

I don’t think we’ll be adding CommonJS-based plugins as we’re trying to migrate to Webpack 2 and specifically its ES modules implementation.

@wasifhyder
Copy link

@cdaringe - Thanks :)

I just added in this comment for future reference in case anyone looks it up. There's not much information on the web here, and this could count as a potential custom fix for anyone who's willing to accept the tradeoff.

As for PR, @gaearon is right. I think it slows the build slightly as well.

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

Closing this as stale. If this is still a problem I encourage you to figure out which project is responsible for this issue (either Babel or Webpack) and follow up with them.

@gaearon gaearon closed this as completed Jan 8, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 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

5 participants