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

Refactor and improve browser stack trace printing #141

Merged
merged 2 commits into from
Jul 7, 2021

Conversation

calebeby
Copy link
Member

@calebeby calebeby commented Jul 7, 2021

There are three changes to the way from-browser stack traces are printed now:

  1. Paths are always printed as absolute paths now. Jest will only print the code frame for absolute paths. Previously, we were printing absolute paths for the lines that Jest would make code frames for, and relative paths otherwise. But that is more complicated and not necessary, because Jest will rewrite the absolute paths to relative paths anyways.
  2. URL's from node_modules get rewritten. Previously, stack lines similar to http://localhost:2345/@npm/react would be printed as-is (since we don't generate source maps for node_modules). But when that happened, Jest would not be able to parse the stack trace (because it doesn't understand URL's, only file paths), and that would prevent it from displaying a code frame for any line in the error. Now the @npm URL's get rewritten to absolute node_modules paths.
  3. Function names always are displayed if they exist: Previously I had mistakenly thought that when function names (like at FuncName (filePath:0:0)) were one of the things that Jest's error stack parser was not able to recognize. It turns out that it is able to recognize those, so now the code for conditionally displaying the function names depending on code frames has been removed.

Copy link
Member

@Paul-Hebert Paul-Hebert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good! I'll admit I don't have enough context to fully understand the underlying changes, but all of the changed lines look good and make sense in isolation. Let me know if you'd like any manual testing done on any of this.

src/index.ts Outdated Show resolved Hide resolved
@calebeby calebeby merged commit a9ef60c into main Jul 7, 2021
@calebeby calebeby deleted the improve-stack-printing branch July 7, 2021 22:42
@github-actions github-actions bot mentioned this pull request Jul 7, 2021
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.

2 participants