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

react-error-overlay line numbers are wrong since 2.1.5 release #6433

Closed
russelldavis opened this issue Feb 14, 2019 · 13 comments
Closed

react-error-overlay line numbers are wrong since 2.1.5 release #6433

russelldavis opened this issue Feb 14, 2019 · 13 comments
Milestone

Comments

@russelldavis
Copy link

russelldavis commented Feb 14, 2019

To reproduce: throw an error in your react app that results in the error overlay appearing. The line number it shows will be wrong (and thus, also the lines of code that it displays).

This was broken by the switch to eval-source-map in the 2.1.5 release. I confirmed that switching back to cheap-module-source-map fixes the issue.

cc @Timer @jasonLaster @ianschmitz

Environment

Environment Info:

System:
OS: macOS 10.14.3
CPU: x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
Binaries:
Node: 11.6.0 - /usr/local/bin/node
Yarn: 1.12.3 - /usr/local/bin/yarn
npm: 6.5.0 - /usr/local/bin/npm
Browsers:
Chrome: 72.0.3626.109
Firefox: 65.0
Safari: 12.0.3
npmPackages:
react: ^16.8.1 => 16.8.1
react-dom: ^16.8.1 => 16.8.1
react-scripts: ^2.1.4 => 2.1.5
npmGlobalPackages:
create-react-app: Not Found

@Timer
Copy link
Contributor

Timer commented Feb 14, 2019

What do the line numbers appear to correspond to?

@Timer Timer added this to the 2.1.6 milestone Feb 14, 2019
@russelldavis
Copy link
Author

They seem to be offset by an arbitrary number (20 lines in my repro case). I'm guessing something is off in the way react-error-overlay is parsing the new sourcemap format.

@RohovDmytro
Copy link

Confirming same bug within my codebase.

@gaearon
Copy link
Contributor

gaearon commented Feb 15, 2019

I bet a reproducing case would help.

@russelldavis
Copy link
Author

Is anyone not able to repro? AFAICT, triggering the overlay (throw an exception) in any CRA app should do it. If that's not the case, I can dig deeper.

@Timer
Copy link
Contributor

Timer commented Feb 15, 2019

I had tested this on a few apps prior to release and the line numbers appeared correct for me.
If this reproduces in the template for you, can you please provide your lockfile?

@bugzpodder
Copy link

I have a repo case in my app, I can try debug this a bit. It is actually pretty confusing to a user because the line displayed in the error overlay had no issues and the error was 12 lines above.

@bugzpodder
Copy link

I was able to repo pretty easily:

Bootstrapping an app:
yarn create react-app repo

The only change I made:

$ git diff
diff --git a/src/App.js b/src/App.js
index 7e261ca..e2cb9ac 100755
--- a/src/App.js
+++ b/src/App.js
@@ -4,6 +4,7 @@ import './App.css';
 
 class App extends Component {
   render() {
+    console.log(null.length);
     return (
       <div className="App">
         <header className="App-header">

React-error-overlay:

TypeError: Cannot read property 'length' of null
App.render
src/App.js:10:8
   7 | console.log(null.length);
   8 | return (
   9 |   <div className="App">
> 10 |     <header className="App-header">
     |    ^  11 |       <img src={logo} className="App-logo" alt="logo" />
  12 |       <p>
  13 |         Edit <code>src/App.js</code> and save to reload.

Clicking on view compiled:

TypeError: Cannot read property 'length' of null
App.render
webpack-internal:///./src/App.js:37:24
  34 |   className: "App-header",
  35 |   __source: {
  36 |     fileName: _jsxFileName,
> 37 |     lineNumber: 10
     |                  ^  38 |   },
  39 |   __self: this
  40 | }, React.createElement("img", {

Actual source code from chrome, notice that line 37 is the correct line, where as react-dev-tools shows a different source.
screen shot 2019-02-16 at 1 25 37 am

@bugzpodder
Copy link

The difference between chrome source and the react-overlay-source are the following extra lines in chrome:
I tested in firefox as well and its the same issue.

__webpack_require__.r(__webpack_exports__);
/* harmony import */ var react__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! react */ "./node_modules/react/index.js");
/* harmony import */ var react__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(react__WEBPACK_IMPORTED_MODULE_0__);

@Timer
Copy link
Contributor

Timer commented Feb 17, 2019

This all sounds vaguely familiar -- I think it's a bug in webpack. We had to get it fixed for cheap-module-source-map. Maybe we can switch back for the meantime?

@ianschmitz
Copy link
Contributor

ianschmitz commented Feb 17, 2019

I've got #6444 up if that's what we want to do in the short term.

@russelldavis
Copy link
Author

russelldavis commented Feb 17, 2019

Note that switching back will reintroduce #6074. So it's basically a choice between breakpoints being broken and the error overlay being broken. 😦

This one probably affects more people more of the time. But when it breaks, it's pretty easy to figure out and look at the console for the real stack trace. With the breakpoints issue, it's really hard to figure out why they aren't working, and once you do, the solution is more tedious (you have to manually kill and re-run npm start).

In conclusion: ¯\(ツ)

@russelldavis
Copy link
Author

@Timer can you reopen #6074?

@lock lock bot locked and limited conversation to collaborators Feb 23, 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

6 participants