Skip to content

Source maps broken since "redbox-react" #40

New issue

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

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

Already on GitHub? # to your account

Closed
jthomaschewski opened this issue Sep 12, 2015 · 73 comments
Closed

Source maps broken since "redbox-react" #40

jthomaschewski opened this issue Sep 12, 2015 · 73 comments

Comments

@jthomaschewski
Copy link
Contributor

Since commit ecc7518 source maps seems to be broken

The nice big red error stacktrace is shown but every class name gets "ProxyClass", line numbers are wrong and the Chrome inspector does only show the bundled file names.

screen_after

With the previous commit 9e6a666 the big red error stacktrace is gone but Chrome shows correct filenames and line numbers (I intentionally broke App.jsx on line 27)

screen_before

@AdamBrodzinski
Copy link
Collaborator

Hmmm that's weird. I wonder if changing devTool source maps to 'source-maps' here. I'll try to look into this over the weekend.

It looks like redbox has a few issues on their repo, delicate-error-reporter might be a bit more stable?

@jthomaschewski
Copy link
Contributor Author

Wow, quick response :)
I've tried both of your suggestions:

"source-maps" and "eval" in "devtool" setting didn't change anything but maybe there was still some cached sourcemap active or something like that.

delicate-error-reporter looks nice but the stack trace and inspector log does look the same as with redbox

Thanks for looking into this

@AdamBrodzinski
Copy link
Collaborator

No prob 😄

I thought I remember that some of the babel transforms work by wrapping classes... i'm wondering if all the error reporters do this? I guess they would have to wrap everything with a try/catch or something to snatch up the errors.

At any rate I think if you remove the 2nd item in the react-transform array that should take it back to normal.

    // removing this:
     {
        "target": "react-transform-catch-errors",
        // the second import is the React component to render error
        // (it can be a local path too, like "./src/ErrorReporter")
        "imports": ["react", "redbox-react"]
      }

@AdamBrodzinski
Copy link
Collaborator

Ok now i'm more confused.... I fired up my fork of webpack (commit ecc7518) and it's working for me. Perhaps it's some kind of caching issue? I know i've seen what you've mentioned though.

Here's what I have with a debugger and showing a 'paused on caught exception' showing vim line number and sourcemap:
screen shot 2015-09-11 at 10 00 42 pm

This may shed some light? perhaps adding the retainLines in .babelrc would help?

webpack-contrib/transform-loader#9
https://github.com/thaiat/webpack-babel-sourcemap

@jedwards1211
Copy link
Owner

Yeah, this caught me off guard too. ProxyClass is obviously from react-redbox, hence the tryRender method...but I'm not sure what to think yet. I haven't seen this issue in my projects yet...

@jedwards1211
Copy link
Owner

Okay, I was able to reproduce in the latest version of this poject, with source-map also...

@jedwards1211
Copy link
Owner

Without react-redbox and source-map, I was able to put breakpoints in App.jsx. I wonder if react-redbox messes up the source maps?

@AdamBrodzinski
Copy link
Collaborator

Yea this is breaking for me too... I thought it was a problem with the debugger lines... I need some sleep 😆

@gaearon
Copy link

gaearon commented Sep 14, 2015

Oh wait, are you all folks using React.createClass() :-) ?

Then you'll need to put react-display-name Babel plugin before react-transform as shown here.
Sorry it wasn't in the docs!

@gaearon
Copy link

gaearon commented Sep 14, 2015

Or wait, I'm missing the point again. The issue is about filenames, not about ProxyClass. Or is about both? These are unrelated issues. You can fix ProxyClass by babel-react-display-name as I wrote in the previous comment, but this is unrelated to sourcemaps.

@jedwards1211
Copy link
Owner

Ah, cool, thanks @gaearon! I'll close my issue once I've verified that fixes it. Yes, we're still in the dark ages here :) I haven't investigated a way to use the MeteorDataMixin with React.Component classes yet.

@jedwards1211
Copy link
Owner

@gaearon I've spent the past hour or two investigating the source map issue, and I'm still not sure what it is. I can't repro by altering react-transform-boilerplate yet. I'm using webpack-dev-server instead of a custom server.js with webpack middleware, so maybe that's the problem.

@gaearon
Copy link

gaearon commented Sep 14, 2015

Let me take a look too.

@gaearon
Copy link

gaearon commented Sep 14, 2015

What do you folks run? npm run dev? npm run dev-debug?

@jedwards1211
Copy link
Owner

@gaearon wow, thanks. In this project just ./dev in the root directory.

@jedwards1211
Copy link
Owner

I put up a simplified project here: https://github.com/jedwards1211/react-transform-source-map-test

In that project stack traces inside component render()s are affected

@jedwards1211
Copy link
Owner

Actually hang on, let me remove some unnecessary deps from that

@gaearon
Copy link

gaearon commented Sep 14, 2015

Note that the stack trace printed from within App.render links to the Webpack bundle, but stack traces in other locations link to the pre-transpiled source.

What is the expected behavior? I don't fully understand the issue so showing expected / actual behavior would help.

@gaearon
Copy link

gaearon commented Sep 14, 2015

From your initial post screenshot, it doesn't seem that sourcemaps are broken. It just says development.js instead of App.js because that's where the error was unhandled. Because react-transform-catch-errors caught the original error from App.js. I'm not actually sure there's anything we can do to influence this, but let me take another look..

@jedwards1211
Copy link
Owner

Ah, sorry. In react-transform-source-map-test, the unexpected stack trace is:

Error: test inside App.render()
    at App.render (eval at <anonymous> (http://localhost:3000/static/bundle.js:1543:2), <anonymous>:58:21)
    at App.proxiedMethod (eval at <anonymous> (http://localhost:3000/static/bundle.js:1585:2), <anonymous>:44:30)
    at ReactCompositeComponentMixin._renderValidatedComponentWithoutOwnerOrContext (eval at <anonymous> (http://localhost:3000/static/bundle.js:1105:2), <anonymous>:789:34)
    at ReactCompositeComponentMixin._renderValidatedComponent (eval at <anonymous> (http://localhost:3000/static/bundle.js:1105:2), <anonymous>:816:14)
    at ReactPerf.measure.wrapper (eval at <anonymous> (http://localhost:3000/static/bundle.js:769:2), <anonymous>:70:21)
    at ReactCompositeComponentMixin.mountComponent (eval at <anonymous> (http://localhost:3000/static/bundle.js:1105:2), <anonymous>:237:30)
    at ReactPerf.measure.wrapper [as mountComponent] (eval at <anonymous> (http://localhost:3000/static/bundle.js:769:2), <anonymous>:70:21)
    at Object.ReactReconciler.mountComponent (eval at <anonymous> (http://localhost:3000/static/bundle.js:775:2), <anonymous>:38:35)
    at mountComponentIntoNode (eval at <anonymous> (http://localhost:3000/static/bundle.js:1003:2), <anonymous>:248:32)
    at ReactReconcileTransaction.Mixin.perform (eval at <anonymous> (http://localhost:3000/static/bundle.js:817:2), <anonymous>:134:20)

Expected stack trace as correctly prints when I throw an error from App in react-transform-boilerplate:

Error: test
    at App.render (App.js:32)
    at App.proxiedMethod (createPrototypeProxy.js:44)
    at App.tryRender (development.js:32)
    at ReactCompositeComponentMixin._renderValidatedComponentWithoutOwnerOrContext (ReactCompositeComponent.js:789)
    at ReactCompositeComponentMixin._renderValidatedComponent (ReactCompositeComponent.js:816)
    at ReactPerf.measure.wrapper (ReactPerf.js:70)
    at ReactCompositeComponentMixin.mountComponent (ReactCompositeComponent.js:237)
    at ReactPerf.measure.wrapper [as mountComponent] (ReactPerf.js:70)
    at Object.ReactReconciler.mountComponent (ReactReconciler.js:38)
    at mountComponentIntoNode (ReactMount.js:248)

@jedwards1211
Copy link
Owner

I can't tell if source maps are broken or something else about my webpack configuration is wrong.

@gaearon
Copy link

gaearon commented Sep 14, 2015

OK, I think I understand the problem now. You used to have nice filenames the console, but now you see eval at <anonymous> (...). Is this correct?

@jedwards1211
Copy link
Owner

Right. In any case now that I've seen everything works with react-transform-boilerplate, seems doubtful it's an issue with any of your code

@gaearon
Copy link

gaearon commented Sep 14, 2015

Maybe it was 4b0bd6b that introduced the change?

@jedwards1211
Copy link
Owner

I've tried with various devtool options, in react-transform-boilerplate as well, no fundamental difference in the behavior of either project

@jedwards1211
Copy link
Owner

quite the complex mash of tools here, and I don't know enough to have a good idea what's causing this

@jedwards1211
Copy link
Owner

Specifically eval and source-map both work in react-transform-boilerplate but not in my react-transform-source-map-test

@gaearon
Copy link

gaearon commented Sep 14, 2015

Sure, no worries. I know it's a crazy mix. Looking into your test case—thanks for assembling it.

@jedwards1211
Copy link
Owner

Oh, man, I'm so sorry, I forgot to check that. It actually doesn't fix the problem. I thought for sure it had to do with react-transform since it's wrapping the class. Well, obviously it's something else so I'm sorry for wasting your time. Thanks for your help though

@gaearon
Copy link

gaearon commented Sep 14, 2015

Not a problem for me, let's keep looking. Please post your exact webpack, webpack-dev-server and webpack/~/webpack-core/~/source-map versions?

@gaearon
Copy link

gaearon commented Sep 14, 2015

You can try copying babel* and webpack* folders from react-transform-boilerplate if it works for you. If it fixes the problem then you have a bad version of something in the test project.

@jedwards1211
Copy link
Owner

andy@hexagon-sun:~/react-transform-issue$ npm ls webpack
react-transform-source-map-test@0.0.1 /home/andy/react-transform-issue
└── webpack@1.12.1 

andy@hexagon-sun:~/react-transform-issue$ npm ls webpack-dev-server
react-transform-source-map-test@0.0.1 /home/andy/react-transform-issue
└── webpack-dev-server@1.10.1  extraneous

And source-map is 0.4.4.

@jedwards1211
Copy link
Owner

Copied the entire node_modules over from react-transform-boilerplate, still no change

@gaearon
Copy link

gaearon commented Sep 14, 2015

Do you have this problem when using https://github.com/gaearon/react-hot-boilerplate?

@jedwards1211
Copy link
Owner

@gaearon I finally figured it out by transforming react-transform-boilerplate step by step into my project. There are two unrelated causes for the issue (either one can cause the issue by itself):

  • using devtool: 'eval-source-map'
  • I accidentally left out .stack in console.error(new Error('test inside App.render()'))

So in other words when I changed to devtool: 'source-map' or devtool: 'eval' and added .stack to that line, it fixed the problem.

Thanks for your help again, and I hope you didn't have to sacrifice too much time on this. And thanks for all you've done for the React community!

@gaearon
Copy link

gaearon commented Sep 15, 2015

No problem. Every riddle feels impossible to solve until it's solved and feels obvious in retrospect.
:-)

@jedwards1211
Copy link
Owner

@gaearon aha, try this in react-transform-boilerplate:

  • devtool: 'source-map'
  • App.js:
export class App extends Component {
  render() {
    console.error(new Error('test').stack);
    throw new Error('test');
    ...

With these settings redbox displays the bad (i.e. linking to the bundle) stack trace, even though the one printed by console.error is good:
image

@jedwards1211
Copy link
Owner

@gaearon Yup, although it won't be obvious to me until if/when I know how the various devtool options work on a deep enough level to understand why they have these effects.

@gaearon
Copy link

gaearon commented Sep 15, 2015

@jedwards1211 redbox-react just uses error-stack-parser, must be their issue?

@jedwards1211
Copy link
Owner

Yeah, I'll go read their source

@jedwards1211
Copy link
Owner

So they parse error.stack as well (on V8/IE)...I think console.error(error.stack) must work some magic (resolve the source maps)

@gaearon
Copy link

gaearon commented Sep 15, 2015

Definitely possible. However it isn't hard for us to provide filename info to the error reporter. Would you like to make a PR to react-transform-catch-errors to pass filename to <ErrorReporter>? It already should receive filename in options.

@jedwards1211
Copy link
Owner

Sure, let me take a look

@jedwards1211
Copy link
Owner

Or would you prefer passing all properties except components, imports, and locals?

@gaearon
Copy link

gaearon commented Sep 15, 2015

I think just filename is fine; arbitrary options from config aren't being passed anyway right now.

@jedwards1211
Copy link
Owner

i.e.

let {components, imports, locals, ...props} = this.props;
props.error = err;
return React.createElement(ErrorReporter, props);

@jedwards1211
Copy link
Owner

okay, sounds good

@jedwards1211
Copy link
Owner

@gaearon
Copy link

gaearon commented Sep 15, 2015

Doing this in plugin is better because we know filename there—we don't have to guess, and devtool doesn't matter.

@jedwards1211
Copy link
Owner

Right, I'm just thinking about the fact that it won't work for the entire stack trace, just the deepest module. In any case it would be nontrivial to use that package right now, and it only supports chrome at the moment

@jedwards1211
Copy link
Owner

@JBBr @AdamBrodzinski I just changed the devtool to eval and pushed. Feel free to use eval-source-map in your own work, but note that this issue will rear its head if you do.

@AdamBrodzinski
Copy link
Collaborator

@jedwards1211 Ah thanks for the heads up! What a crazy bug! I'm using eval currently anyway since it seems to be the fastest reloads. Thanks!!

@jthomaschewski
Copy link
Contributor Author

Thanks for debugging this issue.
Though for me using eval-source-map without redbox-react is still the better choice, as I get correct line numbers for my original files and hot reload is still fast (much faster as with source-map).

eval + redbox-react prints wrong line numbers for me - but at least filenames are correct now 👍

@jedwards1211
Copy link
Owner

@JBBr I understand -- for the default in this skeleton I just want to use the config that's least likely to make people open new issues.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants