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

Debugging in VS Code highlights wrong code line #6044

Closed
raiskila opened this issue Dec 15, 2018 · 21 comments · Fixed by #7022
Closed

Debugging in VS Code highlights wrong code line #6044

raiskila opened this issue Dec 15, 2018 · 21 comments · Fixed by #7022

Comments

@raiskila
Copy link
Contributor

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes

Which terms did you search for in User Guide?

VS Code debugging

Environment

  System:
    OS: macOS 10.14.2
    CPU: x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
  Binaries:
    Node: 10.13.0 - ~/.nvm/versions/node/v10.13.0/bin/node
    npm: 6.4.1 - ~/.nvm/versions/node/v10.13.0/bin/npm
  Browsers:
    Chrome: 71.0.3578.98
    Firefox: 63.0.1
    Safari: 12.0.2
  npmPackages:
    react: ^16.6.3 => 16.6.3 
    react-dom: ^16.6.3 => 16.6.3 
    react-scripts: 2.1.1 => 2.1.1 
  npmGlobalPackages:
    create-react-app: Not Found

Steps to Reproduce

TL;DR: Debug a CRA app in the VS Code debugger and break on a breakpoint or debugger; statement. When clicking on a stack frame in the call stack that's inside react-dom.development.js, the highlighted line of code is wrong.

  1. Install VS Code 1.30.0 or 1.31.0-insider and the Debugger for Chrome extension version 4.11.1.
  2. npx create-react-app debugging-test
  3. Copy the suggested launch.json file from CRA documentation into a newly created .vscode directory inside the project.
  4. Add debugger; to the beginning of the render() method in App.js.
  5. npm start
  6. Hit F5 in VS Code to launch Chrome and start debugging.
  7. After the debugger pauses, select the second-topmost stack frame (finishClassComponent in react-dom.development.js). Observe the highlighted line of code.

Expected Behavior

The correct line of code inside the finishClassComponent function is highlighted, like in Chrome Dev Tools:

screen shot 2018-12-15 at 4 12 30 am

Actual Behavior

An incorrect line of code is highlighted.

screen shot 2018-12-15 at 4 13 07 am

This issue is related to source mapping.

The number of the highlighted line is the same in Chrome Dev Tools and VS Code, but the source code displayed in Chrome Dev Tools, which is derived from source maps, is different. react-dom.development.js has 20,749 lines of code in Chrome, but the actual file shown in VS Code has 19,727 lines.

The extent to which this issue duplicates #5319 is unclear to me. It's not about testing, and doesn't seem to relate to breakpoint location. Rather it's about the code reconstructed from the source maps being different from the actual source and the VS Code debugger being tripped by this. The problem can also be reproduced by calling some code in react-dom and manually stepping into it.

For what it's worth, I tried adding the "disableOptimisticBPs": true option suggested in #5319 to launch.json, but it doesn't seem to be a valid option when "type" is "chrome".

Reproducible Demo

Repro repo

@justingrant
Copy link
Contributor

I'm seeing the same issue. This bug makes VSCode useless for debugging any node_modules code, because you won't be able to step-through, use breakpoints, or do anything that depends on accurately knowing line numbers of code in the debugger. Huge problem.

I did a brief investigation of the issue and here's what I noticed:

  1. As @jraiskila noted, the problem is that different code is at different line numbers between Chrome Dev Tools and VSCode.

  2. The difference in line numbers seems to be entirely caused by differences in whitespace handling. The code is the same in both cases, but the code shown in Chrome has much more extra whitespace than the file stored on disk. For example, look at the top of the react-dom-development.js file in Chrome vs. the VS-Code Debugger (using the OPs repro steps above):

Chrome:

/** @license React v16.6.1
 * react-dom.development.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
'use strict';

if (process.env.NODE_ENV !== "production") {
  (function () {
    'use strict';

    var React = require('react');

    var _assign = require('object-assign');

    var checkPropTypes = require('prop-types/checkPropTypes');

    var scheduler = require('scheduler');

    var tracing = require('scheduler/tracing');

And here's the VSCode version, which is the actual file stored on disk: (FWIW, this is also what you'll see if you use Chrome Dev Tools to open the react-dom.development.js file on disk)

/** @license React v16.6.1
 * react-dom.development.js
 *
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

'use strict';



if (process.env.NODE_ENV !== "production") {
  (function() {
'use strict';

var React = require('react');
var _assign = require('object-assign');
var checkPropTypes = require('prop-types/checkPropTypes');
var scheduler = require('scheduler');
var tracing = require('scheduler/tracing');
  1. It's not just line numbers that are different, it's also intra-line whitespace. The on-disk file has less indentation than the code shown in Chrome. This is less impactful than line numbering, but I assume that this would break columnar breakpoints. Example:

On-disk:

var React = require('react');

Chrome dev tools:

    var React = require('react');
  1. All of the above was reproed on a Mac. At first I was suspicious that the cause might be PC-vs-Unix line endings issues, but given that the entire software stack here including Babel, CRA, and Chrome are all developed on non-Windows environments, I doubt that this is the cause.

@roblourens
Copy link

roblourens commented Dec 31, 2018

This could probably be moved to the vscode repo. This is happening because the file on disk is bundled (and reformatted, I guess) with its new content included in the sourcemap. But the path for the file in the sourcemap is just the path to the original file on disk. vscode thinks it has located the real file, so it shows you the file on disk which has the wrong line numbers. This is sort of a longstanding issue and I don't have a good fix right now.

A workaround on create-react-app's side would be to not do whatever transformations it's doing on this file, since they seem to only be formatting-related. The file actually ends up bigger as a result. But it's not something that CRA should "have" to do.

@justingrant
Copy link
Contributor

Thanks @roblourens. If this is a longstanding issue in VSCode, is there already a VSCode issue in GitHub that I can watch? If not, I'll create one.

What I don't understand is why Chrome's debugger doesn't have the same problem but VSCode does have the problem. Is the reason that VSCode (unlike Chrome) lacks the ability, during debugging, to show HTTP responses that don't correspond to an existing source file on disk? If not, what's the root cause?

Regardless of root cause, it sounds like fixing this on the VSCode side is unlikely to happen soon, so it seems like the most logical fix is to, as you suggested, change create-react-app to "not do whatever transformations it's doing on this file, since they seem to only be formatting-related". Especially if those transformations are making create-react-app source files bigger than they need to be!

@roblourens
Copy link

I don't know of an existing issue that covers exactly this so you could create one, but I'm not likely to make any changes here in the near future. From my perspective, vscode is doing the right thing and the sourcemap is wrong.

What I don't understand is why Chrome's debugger doesn't have the same problem but VSCode does have the problem

Chrome devtools only shows whats in the sourcemap. And if vscode thinks it has found the real source file on disk, then it uses that. I think that's the correct thing to do most of the time.

change create-react-app to "not do whatever transformations it's doing on this file, since they seem to only be formatting-related"

I think this would be the best fix, but I don't know where to start looking to see what it's actually doing here.

@justingrant
Copy link
Contributor

@raiskila @roblourens - I filed a new issue #6296 to simplify the repro by removing VSCode from the repro. Hopefully this will make it easier for non-VSCode users to reproduce and fix the problem!

@stale
Copy link

stale bot commented Feb 28, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Feb 28, 2019
@justingrant
Copy link
Contributor

Not stale!

@stale stale bot removed the stale label Feb 28, 2019
@ziriax
Copy link

ziriax commented Mar 6, 2019

Hitting the same problem here...

@thiago-qat
Copy link

facing the same issue here

@bflorat
Copy link

bflorat commented Mar 25, 2019

same here !

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@justingrant
Copy link
Contributor

Hey! I figured out the cause of this problem and built a PR to fix it. Full details about the problem and the solution are in #7022, but here's a quick TL;DR:

  • The cause was CRA configuring Babel with sourceMaps: false. This setting apparently causes Babel not to omit sourcemaps but instead to emit bad sourcemaps where sourcesContent (the inline code inside the sourcemap file) and the row/col mappings don't match the original source code.
  • Changing it to sourceMaps: true (and adding inputSourceMaps: true too) fixes the problem so that sourcesContent and row/col mappings will match the original source.
  • The problem was only showing up in VSCode (and not in Chrome debugger) because Chrome only uses sourcesContent which was matching the sourcemap's row/col mappings. VSCode (which uses the original source files if they're pointed to by the sources array in the sourcemap) was hosed because the original source didn't match the sourcemap. FYI @roblourens. I wonder if VSCode should compare the source file to the sourcesContent content, and if they don't match, should show sourcesContent instead, so that even if Babel or webpack is misconfigured, the user will at least see valid code?
  • If you don't want to wait until the PR is merged and the next CRA release is cut, you can just edit node_modules/react-scripts/config/webpack.config.js yourself-- it's an easy change. Replace this code:
    // If an error happens in a package, it's possible to be
    // because it was compiled. Thus, we don't want the browser
    // debugger to show the original code. Instead, the code
    // being evaluated would be much more helpful.
    sourceMaps: false,

    with this code:
                // Babel sourcemaps are needed for debugging into node_modules
                // code.  Without the options below, debuggers like VSCode
                // show incorrect code and set breakpoints on the wrong lines.
                sourceMaps: shouldUseSourceMap,
                inputSourceMap: shouldUseSourceMap,

@stale stale bot removed the stale label May 9, 2019
@stale
Copy link

stale bot commented Jun 8, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jun 8, 2019
@justingrant
Copy link
Contributor

Not stale. PR #7022 to fix this issue has been ready for a month. Unfortunately there hasn't been any maintainer feedback on that PR yet, so I'm not sure if maintainers are busy or just don't agree with the fix. It might help attract attention if there are more upvotes.

@sonnenhohl @antzhdanov @microdimmer @mungojam @seanlaff @Ninjef @bbthorntz @Sterv @tkdeshpande @roblourens @ziriax @alexkim205 @ll-aashwin-ll - want to add your thumbs-up to #7022?

@stale
Copy link

stale bot commented Jul 8, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jul 8, 2019
@justingrant
Copy link
Contributor

Not stale. Waiting for #7022.

@stale stale bot removed the stale label Jul 8, 2019
@christopher-j-olsen
Copy link

I've also been experiencing this issue. The workaround given by @justingrant works, but I'd love it if this bug were fixed (I'm rooting for that pull request :-p)

@stale
Copy link

stale bot commented Aug 21, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Aug 21, 2019
@mungojam
Copy link

Not stale, see above

@stale stale bot removed the stale label Aug 21, 2019
@sghosh-discovery
Copy link

Not stale.

@json2d
Copy link

json2d commented Sep 10, 2019

@justingrant nailed it

if you're using react-app-rewired just add this to your overrides:

/* config-overrides.js */

const shouldUseSourceMap = process.env.GENERATE_SOURCEMAP !== 'false';

module.exports = function override(config, env) {

  // fixes dependency source maps
  const babelLoader = config.module.rules[2].oneOf[2] // hardcoded indices according to ejected webpack.config.js

  babelLoader.options.sourceMaps = shouldUseSourceMap
  babelLoader.options.inputSourceMap = shouldUseSourceMap

  return config;
};

works with react-scripts@2.1.2, @3.1.1 and probably others

@lock lock bot locked and limited conversation to collaborators Oct 15, 2019
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 a pull request may close this issue.

10 participants