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

Create sourcemaps for easier debugging #996

Merged
merged 5 commits into from
Feb 24, 2020

Conversation

molant
Copy link
Contributor

@molant molant commented Feb 20, 2020

Summary:

I really need all the help I can get when coding. Having breakpoints in TypeScript instead of JavaScript is really high on my list. Otherwise I always end up modifying the JS and then erasing all changes in the next build because I was modifying the wrong place 😅

The only way I've found to make this work in this project is by creating sourcemaps.
Here's an example of it running on VS Code:

TS breakpoints working on the project

What it does:

  1. Create a valid .map for each file in the build process
  2. Append //# sourceMappingURL=FILE.map to each JS generated file
  3. Ignore all .map files during the publishing process so the package doesn't increase in size. There were some packages that didn't have a files property in package.json. I have not ignored in those cases.

To make this work from VS Code you just need a regular node configuration like in the image above or the following:

{
  "version": "0.2.0",
  "configurations": [  
    {
      "type": "node",
      "request": "launch",
      "name": "CLI",
      "skipFiles": ["<node_internals>/**"],
      "program": "${workspaceFolder}/packages/cli/build/bin.js",
      "args": ["doctor"],
      "console": "integratedTerminal",
      "cwd": "${workspaceFolder}"
    }
  ]
}

It does not require setting up the sourceMaps or outFiles properties.

In this case I'm running the CLI package with the doctor parameter (args). This file is currently ignored by .gitignore even though there are some files from .vscode that are committed.

Happy to contribute this launch.json or try to come up with something more generic if wanted.

Test Plan:

N/A

@thymikee
Copy link
Member

Cool! This broke tests. Can you have a look?

@molant
Copy link
Contributor Author

molant commented Feb 20, 2020

Yes, I'm trying to figure out why and which ones to look at 😅
By looking at other PRs it looks like the new failing ones are lts-tests and unit-tests. Is it safe to ignore the other ones?

@thymikee
Copy link
Member

We've recently fixed Windows tests on Azure, so only e2e-tests fail now

@molant
Copy link
Contributor Author

molant commented Feb 20, 2020

@thymikee I'm looking into it what I'm seeing is really similar to this comment:

If I change jest.config.js and add the following:

collectCoverageFrom: [
  'packages/**/src/**/*.ts',
  '!packages/cli-types/src/**/*.ts',
],

everything passes but the output will get coverage for even for the files that have not been run in the tests (quite a few).

From removing files it looks like the issue is under /packages/cli/src/tools/config. If I delete those tests everything passes, but if I add even one things start failing.

Any idea of what could be going on?

@molant molant force-pushed the fix/breakpoints branch 2 times, most recently from e396f98 to 1e79760 Compare February 20, 2020 23:54
@molant
Copy link
Contributor Author

molant commented Feb 21, 2020

Looks like windows failed because of ENOTEMPTY and unrelated to the above.

I don't think it's related to the previous error and this one could maybe be avoid by increasing maxBusyTries` to something greater than the default 3.

@thymikee
Copy link
Member

Can we rebase this PR to get green CI?

@molant
Copy link
Contributor Author

molant commented Feb 24, 2020

Can we rebase this PR to get green CI?

@thymikee sure!

@molant
Copy link
Contributor Author

molant commented Feb 24, 2020

Everything is ✅ now 🥳

@thymikee thymikee merged commit be0335b into react-native-community:master Feb 24, 2020
@molant molant deleted the fix/breakpoints branch February 24, 2020 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants