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

CRA's sourcemaps don't match code in node_modules, which breaks IDE debugging #6296

Closed
justingrant opened this issue Jan 28, 2019 · 13 comments · Fixed by #7022
Closed

CRA's sourcemaps don't match code in node_modules, which breaks IDE debugging #6296

justingrant opened this issue Jan 28, 2019 · 13 comments · Fixed by #7022

Comments

@justingrant
Copy link
Contributor

justingrant commented Jan 28, 2019

React source code inside sourcemaps generated by create-react-app adds many extra blank lines and extra indentation compared with React's node_modules source. Because line and column numbers don't match between source maps and actual source, debuggers like VSCode don't accurately set breakpoints inside React source and can't accurately step into React source. Debuggers like Chrome are not affected because they use the sitemap source (not the on-disk files) to set breakpoints.

This issue is the cause of #6044 that @raiskila filed 6 weeks ago. I created a new issue with a new repro to simplify the repro case, to remove the need to install VSCode to reproduce the problem, and to clarify that all file-based debuggers will be affected (not just VSCode).

It's possible that the root cause of this problem might be webpack/webpack#8302, although I don't know enough about webpack to know for sure one way or the other. It's also possible that the problem is caused by create-react-app's webpack configuration. It's also possible that the problem might be in how React itself is transforming its source code before publishing generated source files like react-dom.development.js to NPM.

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes. Well, really N/A because the repro starts with npm install -g npm@latest and npx create-react-app so there are no deps to recover!

NPM version is the latest: 6.7.0

Which terms did you search for in User Guide?

sourcemap sourcemaps "source map" "source maps" mismatch whitespace line breakpoint

Environment

Environment Info:

  System:
    OS: macOS 10.14.2
    CPU: x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
  Binaries:
    Node: 10.14.2 - /usr/local/bin/node
    npm: 6.7.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 71.0.3578.98
    Firefox: 63.0.1
    Safari: 12.0.2
  npmPackages:
    react: ^16.7.0 => 16.7.0
    react-dom: ^16.7.0 => 16.7.0
    react-scripts: 2.1.3 => 2.1.3
  npmGlobalPackages:
    create-react-app: Not Found

Steps to Reproduce

1. Create a new app

npm install -g npm@latest
npx create-react-app sourcemap-bug
cd sourcemap-bug
npm start

2. Load the sourcemap and extract code from it

/** @license React v16.7.0\n * react-dom.development.js\n *\n * Copyright (c) Facebook, Inc. and its affiliates.\n *\n * This source code is licensed under the MIT license found in the\n * LICENSE file in the root directory of this source tree.\n */\n'use strict';\n\nif (process.env.NODE_ENV !== "production") {\n (function () {\n 'use strict';\n\n var React = require('react');\n\n var _assign = require('object-assign');\n\n var checkPropTypes = require('prop-types/checkPropTypes');\n\n var scheduler = require('scheduler');\n\n var tracing = require('scheduler/tracing');\n /**\n * Use invariant() to assert state which your program assumes to be true.\n *\n * Provide sprintf-style format (only %s is supported) and arguments\n * to provide information about what broke and what you were\n * expecting.\n *\n * The invariant message will be stripped in production, but the invariant\n * will remain to ensure logic does not differ in production.\n */\n\n\n var validateFormat = function validateFormat() {};\n\n {\n validateFormat = function validateFormat(format) {\n if (format === undefined) {\n throw new Error('invariant requires an error message argument');\n }\n };\n }\n\n

  • 2.4 Now paste that code into an editor and replace all instances of \n with a newline and \" with a regular single quote. You should end up with code like this:
/** @license React v16.7.0
 * 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');
    /**
     * Use invariant() to assert state which your program assumes to be true.
     *
     * Provide sprintf-style format (only %s is supported) and arguments
     * to provide information about what broke and what you were
     * expecting.
     *
     * The invariant message will be stripped in production, but the invariant
     * will remain to ensure logic does not differ in production.
     */


    var validateFormat = function validateFormat() {};

    {
      validateFormat = function validateFormat(format) {
        if (format === undefined) {
          throw new Error('invariant requires an error message argument');
        }
      };
    }

3. Compare the sourcemap code to actual source in node_modules/react-dom/cjs/react-dom.development.js:

/** @license React v16.7.0
 * 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');

/**
 * Use invariant() to assert state which your program assumes to be true.
 *
 * Provide sprintf-style format (only %s is supported) and arguments
 * to provide information about what broke and what you were
 * expecting.
 *
 * The invariant message will be stripped in production, but the invariant
 * will remain to ensure logic does not differ in production.
 */

var validateFormat = function () {};

{
  validateFormat = function (format) {
    if (format === undefined) {
      throw new Error('invariant requires an error message argument');
    }
  };
}

Expected Behavior

Source is the same, which is required for VS Code's debugger breakpoints and step-into to work properly.

Actual Behavior

The "sourcemap source" has many extra blank lines compared with the "node_modules source". As a result of these whitespace differences, VSCode breakpoints won't work and VSCode will set focus to the wrong code line when stepping into React's source code.

Also, the "sourcemap source" is indented differently from the "node_modules source". This breaks columnar breakpoints in VSCode.

Note that the Chrome debugger only uses the "sourcemap source" (and ignores the actual source files) so it's not affected by this problem by default. However, if you use a Chrome devtools workspace to actually load the on-disk source files, it's possible that Chrome debugging may break too.

Refer to #6044 for screenshots demonstrating the impact on VSCode users. This issue is narrowly defined to highlight the root cause of #6044 (the mismatch in whitespace between sourcemaps code and node_modules code) without requiring VSCode to reproduce the problem.

Reproducible Demo

Repro is trivial and the problem reproduces with every create-react-app project so I'm assuming that a hosted repro is not needed. If this is a bad assumption, let me know and I'll post a repro project.

@justingrant justingrant changed the title React's source code differs between sourcemaps vs. node_modules. This breaks VSCode debugging. The code in CRA's sourcemaps doesn't match code in node_modules, which breaks IDE debugging Feb 1, 2019
@justingrant justingrant changed the title The code in CRA's sourcemaps doesn't match code in node_modules, which breaks IDE debugging CRA's sourcemaps don't match code in node_modules, which breaks IDE debugging Feb 7, 2019
@justingrant
Copy link
Contributor Author

justingrant commented Feb 7, 2019

FWIW, I tested every possible value of webpack's devtool setting in https://webpack.js.org/configuration/devtool/. None of them worked. :-(

Here's the process I used for testing on a CRA app:

  1. manually edit the devtool value in node_modules/react-scripts/config/webpack.config.js
  2. npm start
  3. started VSCode debugger
  4. set a breakpoint at ReactDOM.render in my own code
  5. Repeatedly do "step into" in the debugger until it gets into React's code in node_modules/react/cjs/react.development.js
  6. By comparing to the call stack, verified that the debugger is showing the wrong line in the wrong method.
  7. Stop debugger
  8. CTRL+C in the terminal to quit the dev server started by npm start
  9. Pick another devtool value and repeat from step 1

@russelldavis
Copy link

@justingrant not sure if it will make a difference to this particular bug, but note that the config for webpack's sourcemap creation was recently changed. That change fixes a different sourcemap related bug (see #6074 (comment)); it might be worth seeing how it affects this one.

@stale
Copy link

stale bot commented Mar 11, 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 Mar 11, 2019
@justingrant
Copy link
Contributor Author

not stale

@stale stale bot removed the stale label Mar 11, 2019
@gordonmleigh
Copy link

@justingrant the "whitespace handling" is because the CRA webpack config runs node_modules through a limited babel transformation to support ES6 packages after long and concerted demand from users. The "bug" this issue relates to is actually another opinionated and deliberate design choice (#5096 by @Timer) which you can't get out of unless you fork or eject. FWIW I think that options like this should be controllable without an eject.

To save you the hassle of finding it, this code is now here.

@gordonmleigh
Copy link

To get this to work, you need to change two things.

In the loader definition for node_modules change the value for sourceMaps to true (here).

Before the main loaders (around here) add the following:

{
  test: /\.m?js/,
  enforce: 'pre',
  use: ['source-map-loader'],
},

This is so that files in node_modules that have already been transpiled, and have source maps already, are picked up by webpack to show the original source. Remember to add source-map-loader to your package.json!

@stale
Copy link

stale bot commented Apr 20, 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 Apr 20, 2019
@stale
Copy link

stale bot commented Apr 25, 2019

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Apr 25, 2019
@lock lock bot locked and limited conversation to collaborators Apr 30, 2019
@heyimalex heyimalex reopened this May 9, 2019
@stale stale bot removed the stale label May 9, 2019
@facebook facebook unlocked this conversation May 9, 2019
@justingrant
Copy link
Contributor Author

justingrant commented May 9, 2019

PR #7022 is (hopefully) the solution for this issue. Please go over there for more info about the root cause and proposed solution for this problem. And while you're there, vote for it with a thumbs up! ;-)

@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 Author

not stale. still waiting for PR #7022 to get some attention.

@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 Author

Not stale. Waiting for #7022.

@stale stale bot removed the stale label Jul 8, 2019
@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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants