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

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

Closed
ryan-mars opened this issue Oct 5, 2018 · 55 comments · Fixed by #6484
Closed

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

ryan-mars opened this issue Oct 5, 2018 · 55 comments · Fixed by #6484

Comments

@ryan-mars
Copy link
Contributor

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

@Timer Timer added this to the 2.x milestone Oct 5, 2018
@ryan-mars
Copy link
Contributor Author

ryan-mars commented Oct 5, 2018

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
Copy link
Contributor

gaearon commented Oct 5, 2018

@auchenberg Can you please look at this?

@auchenberg
Copy link
Contributor

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
Copy link

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
Copy link
Contributor Author

ryan-mars commented Oct 9, 2018

@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
Copy link

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
Copy link

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
Copy link

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
Copy link

arizonatribe commented Oct 10, 2018

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
Copy link

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
Copy link

It seems that retainLines was removed because of a Babel bug that gives bad output with that option. jestjs/jest#5326 The upstream bug has been open for awhile: babel/babel#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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

ryan-mars commented Oct 16, 2018

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
Copy link
Contributor Author

This issue seems related. jestjs/jest#6683

@roblourens
Copy link

That sounds like a different issue entirely?

@arizonatribe
Copy link

arizonatribe commented Oct 17, 2018

@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).

@stale
Copy link

stale bot commented Jan 18, 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
Copy link

stale bot commented Jan 26, 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 Jan 26, 2019
@ianschmitz ianschmitz reopened this Jan 30, 2019
@IAmNatch
Copy link

IAmNatch commented Feb 2, 2019

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
Copy link
Contributor

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
Copy link

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
Copy link

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

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

@ianschmitz ianschmitz reopened this Feb 10, 2019
@stale stale bot removed the stale label Feb 10, 2019
@ianschmitz
Copy link
Contributor

We will be releasing a new version including the change made in #5060 shortly. I hope this may improve some of these issues you're experiencing.

@justingrant
Copy link
Contributor

justingrant commented Feb 10, 2019

@ianschmitz - unfortunately I don't think this will help because I've already tested every possible devtool option (see #6296 (comment)) 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
Copy link

"env": { "CI": true },
"disableOptimisticBPs": true

Has solved the problem for me !

@ryan-mars
Copy link
Contributor Author

ryan-mars commented Feb 21, 2019

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

@johnrazeur
Copy link

@ryanwmarsh yes

@ryan-mars
Copy link
Contributor Author

@johnrazeur I just tested it and created a PR. Thank you!

@roblourens
Copy link

What does "env": { "CI": "true" }, do?

@codenamekt
Copy link

This works great. Is this where we spam the PR with 👍 @ryanwmarsh

@danielkatz
Copy link

"env": { "CI": true },
"disableOptimisticBPs": true

Has solved the problem for me !

Thanks for the solution!
For me disableOptimisticBPs: true alone did the trick.

@mvgiacomello
Copy link

For whoever is having issues debugging Webdriver.IO, the "disableOptimisticBPs": true in the launch file does the trick. Example project here.

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.