facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.73k stars 26.85k forks source link

CRA2 recommended config hits random break points in VS Code #5319

Closed ryan-mars closed 5 years ago

ryan-mars commented 6 years ago

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes

npm --version 6.4.1 yarn --version 1.10.1

Which terms did you search for in User Guide?

"vs code" debugger breakpoints

Environment

  System:
    OS: macOS 10.14
    CPU: x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 10.10.0 - ~/.nvm/versions/node/v10.10.0/bin/node
    Yarn: 1.10.1 - ~/.nvm/versions/node/v10.10.0/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v10.10.0/bin/npm
  Browsers:
    Chrome: 69.0.3497.100
    Safari: 12.0
  npmPackages:
    react: ^16.5.2 => 16.5.2 
    react-dom: ^16.5.2 => 16.5.2 
    react-scripts: 2.0.4 => 2.0.4 
  npmGlobalPackages:
    create-react-app: 2.0.2

Steps to Reproduce

  1. Install VS Code 1.27.2 or 1.26.1
  2. npx create-react-app debug-cra2
  3. Add recommended Debug CRA Tests snippet to .vscode/launch.json
  4. Add the snippet below to src/App.test.js
  5. Set breakpoint on line 13 console.log({ foo })
  6. Hit F5 and debug

Snippet for App.test.js

it('should find the breakpoint', () => {
  const foo = 'bar'
  console.log({ foo })
  expect(foo).toBe('bar')
})

it('should find the other breakpoint', () => {
  const foo = 'bar'
  console.log({ foo })
  expect(foo).toBe('bar')
})

Expected Behavior

Execution stops on line 13

Actual Behavior

Execution stops on line 6 or 7, it's inconsistent.

Reproducible Demo

https://github.com/ryanwmarsh/debug-cra2

ryan-mars commented 6 years ago

For added information...

Using the Chrome inspector works fine.

Steps to reproduce:

  1. Add the snippet below to package.json
  2. Add debugger statement to line 13
  3. npm run test:debug
  4. Open chrome://inspect
  5. Resume script execution
  6. Observe that execution pauses on debugger

package.json snippet:

"scripts": {
 "test:debug": "react-scripts --inspect-brk test --no-cache --runInBand --env=jsdom"
}

I'm not sure if this is a VS Code bug or a problem with CRA's recommended launch.json settings.

gaearon commented 6 years ago

@auchenberg Can you please look at this?

auchenberg commented 6 years ago

This sounds like a sourcemapping problem that causing the breakpoint to be set on the wrong line. Adding debugger should also make the VS Code debugger break. I'll add debugging this to my backlog.

Question @ryancogswell Can you set breakpoints with Chrome DevTools using their Node.js debugging? Breakpoint from the gutter.

arizonatribe commented 6 years ago

I'm not a VSCode user but I was troubleshooting this issue two weeks ago for devs on my team (on an ejected create-react-app build) and I couldn't understand why it worked with react-scripts (prior to v2 release) but didn't on mine. I configured the launch.json like this for CRA (which worked):

{
    "version": "0.2.0",
    "configurations": [
      {
        "name": "Debug Jest (CRA)",
        "type": "node",
        "request": "launch",
        "runtimeExecutable": "react-scripts",
        "runtimeArgs": [
          "--inspect-brk"
        ],
        "args": [
          "test",
          "--runInBand",
          "--no-cache",
          "--watch",
          "--env=jsdom",
          "--coverage"
        ],
        "env": {
          "NODE_ENV": "test"
        },
        "cwd": "${workspaceRoot}",
        "protocol": "inspector",
        "console":"integratedTerminal",
        "internalConsoleOptions":"neverOpen"
      }]
}

But on my build it didn't work. After a lot of side-trails and meandering through VSCode and Jest issue threads I finally got it working by downgrading my project's version of Jest from 23.4.x to 22.2.0, and it started working.

ryan-mars commented 6 years ago

@arizonatribe Thank you for taking the time to figure this out and share it. I lost two days trying to fix this and the one thing I failed to try is downgrading Jest.

@auchenberg Do you think I should open an issue with Jest?

roblourens commented 6 years ago

tl;dr: a workaround is that breakpoints set after launching work.

I can repro this, I tried downgrading jest in the test repo, but it doesn't make a difference for me? Could one of you share a test repo where it does work with jest downgraded? Or if it's easier just set "trace": true in your launch config, run it again, and share the log file.

Here's the problem: In a transpiling scenario you can use the outFiles parameter to point vscode towards the output sourcemaps so it can preload them and resolve your breakpoints to their real locations. But in this scenario, the sourcemaps are generated in memory and don't exist on disk.

So you set a breakpoint in a .js file. VS Code doesn't know whether it's a bp in a script with no sourcemaps, or a transpiled script that will have sourcemaps at runtime. Since it's a .js file, we guess and set breakpoints for that path and line. Anyone using .ts or something else won't see an issue.

Then, jest loads the transpiled script and gives it the same path as the file on disk. It's a match, and your breakpoint hits. But the line numbers don't match between the two scripts, so you've stopped on the wrong line. At the same time, vscode has loaded the sourcemap so any breakpoints you set at this point will bind correctly.

If this is really new in Jest then they might have changed how they name the loaded scripts. If they can name them differently from the source files on disk, that would fix this. Maybe we should provide an option to disable this optimistic bp placement but it should "just work"...

But this scenario where sourcemaps can't be preloaded from disk has always been imperfect in vscode. We would still have a race to load the sourcemap and set the BP before the line of code with the BP executes. Chrome gives us a way to break every time a script loads, giving us a chance to catch up and load sourcemaps. Node doesn't support that unfortunately. We've experimented with guessing the name of the script at runtime, setting a BP for line 1, then checking whether it has sourcemaps. That would help in this scenario but is complex and relies on the name of the script at runtime matching the path on disk.

Yikes sorry for the wall of text...

arizonatribe commented 6 years ago

I'm not sure I can share the code repo without permission, but I'll check and see if it's okay to share the logs.

I forgot one other thing I did was set reatainLines to true and sourceMaps to "inline" in the Babel config (but only for the Babel test env)

roblourens commented 6 years ago

reatainLines to true

That sounds extremely likely to help, how do you do that? I'm not clear on how you override config in a CRA app

arizonatribe commented 6 years ago

I don't believe that part can be customized unless you eject . When the app is ejected, the babelrc: false and configFile: false code are removed.

I know ejecting is not a solution (maintaining an ejected create-react-app based build eats up a good portion of my time and makes me develop an irrational dislike for jest, webpack and and the whole ecosystem of plugin/loader authors that can never stay in-sync), but you might be able to verify locally whether it works with a Jest downgrade to v22 and/or by setting that prop locally in the node_modules/react-scripts/config/jest/babelTransform.js.

const babelJest = require('babel-jest');

module.exports = babelJest.createTransformer({
  presets: [require.resolve('babel-preset-react-app')],
  retainLines: true,
  sourceMaps: 'inline',
  babelrc: false,
  configFile: false,
});

Then you should be able to re-launch the test script in VSCode to see if it's hitting those breakpoints correctly or not.

If that setting is the trick to getting it working (and especially if it works with Jest v23) maybe they'll accept a PR for changing the babelTransform.js file.

roblourens commented 6 years ago

Ok I was able to create a project with create-react-app@1, and I do see that it sets retainLines in node_modules/babel-jest/build/index.js. I think that bringing that back is the best fix. I'll try to do a PR for babel-jest.

roblourens commented 6 years ago

It seems that retainLines was removed because of a Babel bug that gives bad output with that option. https://github.com/facebook/jest/issues/5326 The upstream bug has been open for awhile: https://github.com/babel/babel/issues/7275

I think there are a couple things vscode could do to make this scenario work better but they are not great and I don't want to add more options or modes.

At the moment, setting retainLines: true in node_modules/react-scripts/config/jest/babelTransform.js is a good local workaround, assuming your code doesn't trigger the babel bug.

I'd appreciate any other suggestions, in the meantime I'll think about what vscode can do here...

ryan-mars commented 6 years ago

Not sure if this is the right thing to do here but I submitted babel/babel#8868

I have no idea what I'm doing.

loganfsmyth commented 6 years ago

retainLines is buggy and a best-effort of preserving lines, so there are still cases where the lines it outputs don't match up. It was removed from Jest because it's not something we recommend using at all. While we may fix specific that bug, I'd still consider using it a mistake.

If debuggers are having problems with the sourcemapped output, then that seems like the real issue to investigate. While retainLines may make things work, it is most likely just preventing the underlying problem from manifesting.

ryan-mars commented 6 years ago

No prob. At least I learned how Babel works. 😜Could you point me to whichever (Jest) sourcemap issue covers this so I can contribute a PR? Thanks.

roblourens commented 6 years ago

Besides that it's just a fundamental issue with debuggers. I don't have any other suggestions for Jest/Babel changes. Writing generated files/sourcemaps to disk would be a lot simpler for debuggers but that sounds unlikely to change.

ryan-mars commented 6 years ago

Could we resolve this by having CRA output generated files or sourcemaps for the debugger?

Ironically, when debugging tests in babel-generator (in the hopes of resolving this issue) I found the debugger lands on generated files in babel-generator/lib (as opposed to src) and debugging was predictable.

roblourens commented 6 years ago

I believe that either writing files to disk or just naming the runtime files differently (I assume they are eval'd with sourceURL or similar) would help.

ryan-mars commented 6 years ago

To be clear, this isn't just a VS Code issue. I can't hit breakpoints in Chrome (node inspector) either. It also skips debugger; statements. Again this is only for debugging Jest tests.

I tried modifying node_modules/react-scripts/config/jest/babelTransform.js to add sourceMaps: 'inline' and retainLines: true 😱Neither made any difference.

ryan-mars commented 6 years ago

This issue seems related. https://github.com/facebook/jest/issues/6683

roblourens commented 6 years ago

That sounds like a different issue entirely?

arizonatribe commented 6 years ago

@ryanwmarsh are you seeing that for Jest v23 and v22? I was only able to make breakpoints hit correctly on v22 of Jest, but I'm not 100% sure I know what was causing it. It was a long debugging session and when I finally had versions of Babel, Webpack, Jest, and VSCode that were all playing well together, I didn't investigate further (and probably won't unless someone on our team really needs a new version of Jest for an important feature).

dipunm commented 6 years ago

I believe that either writing files to disk or just naming the runtime files differently (I assume they are eval'd with sourceURL or similar) would help.

How might we go about naming the runtimes differently? I had a quick look, --out-dir is only available to configure via the CLI when writing to disk. I'm not sure where babel "names" the transpiled code but that would help me to understand.

khaled-hossain-code commented 6 years ago

I am unable to hit the breakpoint. why this issue is closed? and what is the configuration file as I don't wanna eject and using cra2 I can't change my jest version

roblourens commented 5 years ago

I added a flag that will make this work a little better. In the next vscode Insiders build you can set "_disableOptimisticBPs": true to prevent vscode from setting breakpoints until a sourcemap has loaded for the script. This will mean that breakpoints will only bind to the correct line, but they probably won't bind before the test actually runs. So you can add a debugger statement to the top of the file to give it some time to load the sourcemap, or rerun the tests in the terminal.

Anyone watching this thread, I'd appreciate if you try it out in tomorrow's Insiders build. It's not a permanent solution but should unblock this at least.

roblourens commented 5 years ago

Has anyone tried this out? Can you comment on whether you it helps, and how the experience compares to using chrome devtools? Need to decide whether to make this part of the official recommended and documented workflow.

ryan-mars commented 5 years ago

@roblourens I just tried debugging CRA2 tests with "_disableOptimisticBPs": true in user settings for the latest insiders. The breakpoint landed about 10 lines off.

It doesn't work in Chrome either. Debugging tests is just flat out broken in CRA.

roblourens commented 5 years ago

It doesn't work in Chrome either.

Above you said it does work in Chrome? That would indicate a different problem than the one I'm trying to work around. It works for me with your original repo.

DiegoBM commented 5 years ago

I have the same problem as @ryanwmarsh, the sourcemap is completely lost when debugging in chrome (the real deal, no tests)

roblourens commented 5 years ago

This issue is specifically about test debugging.

roblourens commented 5 years ago

For anyone using the new flag in vscode Insiders, I've removed the underscore and it will be in the next stable release. I recommend using the flag for debugging tests with Jest/CRA:

"disableOptimisticBPs": true

Also, work is in progress to give Node the ability to pause on script load: https://github.com/nodejs/node/issues/24687

This will help vscode with this issue, because it will be able to pause when your test script is loaded, load the sourcemap, set breakpoints correctly, then continue execution.

mrmckeb commented 5 years ago

Hi @roblourens, is this likely to be in the December VSCode release?

Also, a few users have had unverified breakpoints showing up in VSCode too, which I believe is related to this issue.

jeremypeters commented 5 years ago

Regarding the related #5984, it will break on a debugger statement for me and then if I set a breakpoint that will get hit.

jeremypeters commented 5 years ago

Just for further information, I usually debug regular code in Chrome - I just tried it in VSCode and none of my breakpoints were hit, but debug statements were. I will also occasionally find that breakpoints will not be hit in Chrome either - usually a $ npm run start will fix this.

roblourens commented 5 years ago

The disableOptimisticBPs is in the current Insiders and will be in the next stable release (actually called the November release) next week.

it will break on a debugger statement for me and then if I set a breakpoint that will get hit.

Have you tried with this new flag yet? Still note the caveat above,

This will mean that breakpoints will only bind to the correct line, but they probably won't bind before the test actually runs. So you can add a debugger statement to the top of the file to give it some time to load the sourcemap, or rerun the tests in the terminal.

mrmckeb commented 5 years ago

Thanks @roblourens, and for the correction on the release name (I knew that, I use VSCode daily, but somehow forgot). Thanks again for your work on this.

jeremypeters commented 5 years ago

@roblourens No, I haven't tried the new flag - I'm not using the Insiders version, and it looks like the flag didn't work for the people that tried it.

justingrant commented 5 years ago

@auchenberg @roblourens - is the problem in this issue the same root cause as #6044 that @jraiskila just filed? In that issue, the source shown in Chrome Dev Tools has extra line breaks and extra indentation compared to the same source file on disk (which is what's loaded by VSCode, as well as by Chrome if you load from the Filesystem tab). The code is the same, just newlines & whitespace are different, but line numbers are wrong in VSCode. This causes all kinds of problems when stepping through in the debugger, when looking at call stacks, etc.

Seems like the underlying root cause of that issue (and perhaps this issue too?) is that VSCode assumes that row/col values in the runtime call stack (which are presumably set via sourcemaps) will match the row/col in the actual source file on disk in node_modules. In current CRA2, this assumption isn't true.

I don't see how any breakpoints (or any other debugging actions!) can work properly in VSCode if VSCode can't match the source you're editing to the runtime row/col values provided by the Chrome debugging host.

What I don't understand is the expectations around sourcemaps. Is it expected that sourcemap output should always exactly match a file stored on disk? (Meaning the culprit here is CRA, Babel, and/or Webpack not generating sourcemaps properly.)

Or is it a valid case where sourcemap output and on-disk source won't match, and it's up to the debugger UI to detect this case and to show sourcemap output instead of the on-disk file? (meaning that the problem is VSCode's, because Chrome devtools correctly handles this case while VSCode 1.30 does not.)

roblourens commented 5 years ago

No, it doesn't sound related. This issue is about jest tests specifically. I can take a look at your issue when I have a minute.

stale[bot] commented 5 years ago

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[bot] commented 5 years ago

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.

IAmNatch commented 5 years ago

I downloaded the latest version of Code, created a fresh CRA and used the suggested configuration, as well as the extra setting. Neither of these solution's have resolved the issue.

Any assistance would be greatly appreciated.

justingrant commented 5 years ago

Given that disabling optmistic BPs doesn't seem to fix the problem (at least for some folks), is it possible that there are two separate problems happening here that both contribute to breakpoints set at the wrong line? For example:

Problem #1 is the optimistic breakpoints issue that @roblourens noted above. But is this the only problem?

Problem #2 (#6296) is that webpack is emitting invalid sourcemaps where the code doesn't match the code inside node_modules that was downloaded from NPM. If the sourcemap code doesn't match code files on disk, then VSCode breakpoints will always be wrong for any code that's "downhill" from the first mismatched line in each sourcemap.

I haven't tested whether #6296 also shows up in Jest runs (nor do I even know how to check this given that sourcemaps are in memory!) but if it does, then Rob it might also explain why the problem doesn't show up all the time for everyone. Depending on your configuration, for example, one project might have a single chunk (e.g. 0.chunk.js.map) where the problem happens on Line 10 in the sourcemap, while another project might have 2+ chunks where the first "bad" line in the sourcemap is halfway down each chunk. In this first hypothetical project all breakpoints are bad, while in the second project breakpoints will work fine in half of the total code.

Anyway, if both of these could be contributing to this problem, then what's needed to make progress on #6296? It certainly couldn't hurt to fix sourcemaps for non-Jest debuggers! ;-)

FWIW, webpack itself has apparently been generating invalid sourcemaps for at least 6 months. webpack/webpack#8302. Not sure if that's related to this issue and/or #6296. But that issue might point to the right folks on the webpack project to look into this issue.

roblourens commented 5 years ago

You are right that the first two problems are separate issues. webpack/webpack#8302 is interesting, thanks for pointing that out. I don't know whether it directly relates to #6296 but it doesn't sound like it would help.

stale[bot] commented 5 years ago

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.

justingrant commented 5 years ago

Adding unnecessary comment to get stalebot to re-open this issue.

ianschmitz commented 5 years ago

We will be releasing a new version including the change made in https://github.com/facebook/create-react-app/pull/5060 shortly. I hope this may improve some of these issues you're experiencing.

justingrant commented 5 years ago

@ianschmitz - unfortunately I don't think this will help because I've already tested every possible devtool option (see https://github.com/facebook/create-react-app/issues/6296#issuecomment-461637016) and none of them fixed the problem described in #6296 where the webpack sourcemap content doesn't match the original source files in node_modules.

But regardless I'll definitely try the new release to see if some other commit helps.

johnrazeur commented 5 years ago
"env": { "CI": true },
"disableOptimisticBPs": true

Has solved the problem for me !

ryan-mars commented 5 years ago

@johnrazeur You put those in .vscode/launch.json?

johnrazeur commented 5 years ago

@ryanwmarsh yes