Pr/440 show line in debug #733

victorandcode · 2020-08-08T19:02:41Z

What:
Add the call frame to the output of screen.debug before the prettified DOM

Why:
For easier debugging.
Resolves #440

How:

Create an empty Error and filter it's stack frames to keep only the one we care about
Coloring the message with chalk

Before

After

Here's the code for the test

Checklist:

Documentation added to the
docs site
Tests
Typescript definitions updated
Ready to be merged

codesandbox-ci · 2020-08-08T19:03:48Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6c6df90:

Sandbox	Source
kentcdodds/react-testing-library-examples	Configuration

codecov · 2020-08-08T19:07:20Z

Codecov Report

Merging #733 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #733   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        25    +1     
  Lines          675       702   +27     
  Branches       181       183    +2     
=========================================
+ Hits           675       702   +27

Impacted Files	Coverage Δ
src/get-user-code-frame.js	`100.00% <100.00%> (ø)`
src/pretty-dom.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19901e1...6c6df90. Read the comment docs.

kentcdodds

This looks mostly ok to me (what are the chances we could create an actual codeframe? https://babeljs.io/docs/en/next/babel-code-frame.html)

src/get-user-trace.js

victorandcode · 2020-08-15T14:21:34Z

After doing some research based on the feedback, I believe we have 2 options to implement this feature 🧑‍🏭

If we only want to display the frame like `location/to/my/file.js:10:2`

This can only be achieved in node since we are counting on ignoring external packages inside node_modules. Also, we need the environment (jest), to calculate correctly the line of the error with source mapping. We can do this by using the inNode function and only printing the frame when appropriate.

If we want to display the code frame as suggested by @kentcdodds

This feature can be achieved via @babel/code-frame + node's fs module. I don't see any alternative to do this on the browser, therefore, it would be node exclusive like the previous option.

In order to achieve this, we would need to do some tweaking in the build config to mock those 2 modules I mentioned so these don't cause any trouble in the browser. This is still pending on the PR until we reach a decision.

Let me know if you feel like any of those 2 options is worth pursuing :)

kentcdodds · 2020-08-15T14:32:58Z

I like option 2. I don't think we'd need anything special in config. We could do something like this: https://github.com/testing-library/react-testing-library/blob/master/src/flush-microtasks.js#L28

victorandcode · 2020-08-16T09:11:23Z

src/get-user-code-frame.js

+  const nodeRequire = module && module.require
+  readFileSync = nodeRequire.call(module, 'fs').readFileSync
+  codeFrameColumns = nodeRequire.call(module, '@babel/code-frame')
+    .codeFrameColumns
+} catch {
+  // We're in a browser environment
+  /* istanbul ignore next */


Given that these lines run while the module is loaded, I don't know of a practical way to mock them for a single test to force them to throw. That's why I'm ignoring coverage for now. If you know of a way to do this, I'd appreciate the guidance 🙏

victorandcode · 2020-08-16T09:26:50Z

Option 2 is now implemented and code frames are now displayed 🎉 Please take a look when you have some time. I also tested the UMD build in chrome and it's working fine.

Here's an example:

kentcdodds · 2020-08-16T12:23:14Z

This is fantastic! I'll give the code a deeper look later, but one thing: could we put the codeframe at the bottom?

kentcdodds · 2020-08-16T12:24:27Z

Oh and the line above the codeframe appears to be listing the wrong file, it should show the one that is shown in the codeframe right?

victorandcode · 2020-08-16T12:38:42Z

Oh and the line above the codeframe appears to be listing the wrong file, it should show the one that is shown in the codeframe right?

So actually that line comes from jest, it's where console.log is actually called. I have looked into it but I haven't found a way to prevent it :(

victorandcode · 2020-08-16T13:51:53Z

Oh and the line above the codeframe appears to be listing the wrong file, it should show the one that is shown in the codeframe right?

This has now been fixed. Here's the new version:

kentcdodds · 2020-08-16T15:36:26Z

Super! Let's add another line between the debug output and the line listing where the file is, and let's also make that line muted like the console.log line. Thanks!

victorandcode · 2020-08-16T18:50:50Z

Super! Let's add another line between the debug output and the line listing where the file is, and let's also make that line muted like the console.log line. Thanks!

This is now fixed and looks like this 👀

I still need to take a look at the CI error, it shouldn't be failing since locally it doesn't with the same script 🤔

kentcdodds

This is great! Thank you. Just a few things.

kentcdodds · 2020-08-19T23:15:46Z

src/__tests__/get-user-code-frame.js

+
+test('it returns only client code frame when code frames from node_modules are first', () => {
+  const stack = `Error: Kaboom
+      at Object.<anonymous> (/home/john/projects/projects/sample-error/node_modules/@es2050/console/build/index.js:4:10)


This says john/projects/projects, but the snapshot just says john/projects/ 🤔

On this particular test, that frame should actually be ignored since it's inside node_modules. The one we care about is one line after ${userStackFrame}.
However, the frame that is ignored does have a typo (/projects/projects). I've corrected this and simplified the paths used for all tests to avoid confusion. They all now start with /sample-error instead of /home/john/<etc>

src/__tests__/screen.js

src/get-user-code-frame.js

…me for debugging

…an't be loaded

…t be loaded

…ame into variable

victorandcode · 2020-08-30T09:11:43Z

Hey @kentcdodds, please let me know if the latest changes look ok.

All comments have been addressed but the travis error still puzzles me. It fails in a seemingly unrelated file (role.js) but passes locally.

I also tried temporarily rolling back my changes, creating a very simple commit but the error still happened.

I was wondering if this had happened before

kentcdodds · 2020-08-30T13:03:45Z

Yeah, that's odd. I'm not sure what's up with.

Anyone else want to investigate?

kentcdodds · 2020-09-02T17:31:11Z

Tried it locally myself and everything worked fine. So I'm going to just hope this is an issue with CI for this PR and we'll go ahead and merge it. I'll deal with it if it persists beyond this PR CI issue.

kentcdodds

This is solid. Thanks!

kentcdodds · 2020-09-02T17:34:41Z

Looks like there was a weird issue in CI. Cleared cache and ran again and everything's fine now.

testing-library-bot · 2020-09-02T17:42:03Z

🎉 This PR is included in version 7.24.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

kentcdodds · 2020-09-02T18:56:18Z

@all-contributors please add @victorandcode for code and tests

allcontributors · 2020-09-02T18:56:30Z

@kentcdodds

I've put up a pull request to add @victorandcode! 🎉

victorandcode · 2020-09-02T20:27:06Z

Looks like there was a weird issue in CI. Cleared cache and ran again and everything's fine now.

Thanks for taking a look :)

kentcdodds reviewed Aug 12, 2020

View reviewed changes

eps1lon reviewed Aug 12, 2020

View reviewed changes

src/get-user-trace.js Outdated Show resolved Hide resolved

victorandcode force-pushed the pr/440-show-line-in-debug branch from 2f5ee1d to 74ca2c2 Compare August 15, 2020 13:43

victorandcode requested review from kentcdodds and eps1lon August 15, 2020 14:23

victorandcode force-pushed the pr/440-show-line-in-debug branch from 74ca2c2 to 18fb7dd Compare August 15, 2020 20:20

victorandcode commented Aug 16, 2020

View reviewed changes

victorandcode force-pushed the pr/440-show-line-in-debug branch 2 times, most recently from e36f525 to 439a4d7 Compare August 16, 2020 11:57

victorandcode force-pushed the pr/440-show-line-in-debug branch from 08a4c55 to 32aa265 Compare August 16, 2020 18:46

kentcdodds reviewed Aug 19, 2020

View reviewed changes

victorandcode force-pushed the pr/440-show-line-in-debug branch 2 times, most recently from 5b91123 to 742dbbd Compare August 20, 2020 19:30

victorandcode requested a review from kentcdodds August 20, 2020 19:35

victorandcode added 5 commits August 30, 2020 09:58

feat(get-user-trace): create utility to obtain the client's stack fra…

fe31e34

…me for debugging

feat(pretty-dom): return user stack from pretty-dom

9692ebe

feat(get-user-trace): add coloring to get-user-trace

2c0797b

feat: instead of showing location of debug call, print full code frame

3f02326

feat: conditionally load node dependencies and return empty if they c…

202abcd

…an't be loaded

victorandcode added 6 commits August 30, 2020 09:59

test: ignore coverage of catch when dependencies to render trace can'…

32cd44c

…t be loaded

test: refactor test names to be more expressive and move user code fr…

3be5856

…ame into variable

refactor: rename get-user-trace to get-user-code-frame

514f841

feat: display location on top of code frame

859aa7e

feat: make frame location color dimmer

b108fe0

test: shorten paths used as test data for get user code frame

c47fca6

victorandcode force-pushed the pr/440-show-line-in-debug branch 2 times, most recently from be70a1f to e04c63d Compare August 30, 2020 08:16

victorandcode closed this Aug 30, 2020

victorandcode force-pushed the pr/440-show-line-in-debug branch from e04c63d to 19901e1 Compare August 30, 2020 08:31

victorandcode reopened this Aug 30, 2020

victorandcode force-pushed the pr/440-show-line-in-debug branch 2 times, most recently from e04c63d to d641ec1 Compare August 30, 2020 08:56

feat: remove warnings from get user code frame

6c6df90

victorandcode force-pushed the pr/440-show-line-in-debug branch from d641ec1 to 6c6df90 Compare August 30, 2020 09:01

kentcdodds approved these changes Sep 2, 2020

View reviewed changes

kentcdodds merged commit dfb4aa1 into testing-library:master Sep 2, 2020

testing-library-bot added the released label Sep 2, 2020

allcontributors bot mentioned this pull request Sep 2, 2020

docs: add victorandcode as a contributor #759

Merged

Pr/440 show line in debug #733

Pr/440 show line in debug #733

Uh oh!

Conversation

victorandcode commented Aug 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Aug 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

victorandcode commented Aug 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

If we only want to display the frame like location/to/my/file.js:10:2

If we want to display the code frame as suggested by @kentcdodds

Uh oh!

kentcdodds commented Aug 15, 2020

Uh oh!

victorandcode Aug 16, 2020

Choose a reason for hiding this comment

Uh oh!

victorandcode commented Aug 16, 2020

Uh oh!

kentcdodds commented Aug 16, 2020

Uh oh!

kentcdodds commented Aug 16, 2020

Uh oh!

victorandcode commented Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

victorandcode commented Aug 16, 2020

Uh oh!

kentcdodds commented Aug 16, 2020

Uh oh!

victorandcode commented Aug 16, 2020

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

kentcdodds Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

victorandcode Aug 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

victorandcode commented Aug 30, 2020

Uh oh!

kentcdodds commented Aug 30, 2020

Uh oh!

kentcdodds commented Sep 2, 2020

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Sep 2, 2020

Uh oh!

testing-library-bot commented Sep 2, 2020

Uh oh!

kentcdodds commented Sep 2, 2020

Uh oh!

allcontributors bot commented Sep 2, 2020

Uh oh!

victorandcode commented Sep 2, 2020

Uh oh!

Uh oh!

victorandcode commented Aug 8, 2020 •

edited

Loading

codesandbox-ci bot commented Aug 8, 2020 •

edited

Loading

codecov bot commented Aug 8, 2020 •

edited

Loading

victorandcode commented Aug 15, 2020 •

edited

Loading

If we only want to display the frame like `location/to/my/file.js:10:2`

victorandcode commented Aug 16, 2020 •

edited

Loading