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

Use debug module for logging instead of console.log #314

Closed
alexkrolick opened this issue Jul 10, 2019 · 14 comments
Closed

Use debug module for logging instead of console.log #314

alexkrolick opened this issue Jul 10, 2019 · 14 comments

Comments

@alexkrolick
Copy link
Collaborator

Describe the feature you'd like:

DTL currently provides several debug methods that pretty print the DOM

What if instead of, or in addition to, these methods we added a debug scope using the debug npm module?

https://www.npmjs.com/package/debug

Before:

  • write test
  • get weird output
  • add debug log calls to test

After:

  • DEBUG=dom:query:* yarn test (on changed files or with a specific test regex)

Suggested implementation:

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

@kentcdodds
Copy link
Member

Interesting idea! I wish we'd thought of this before making the debug and log* APIs. We could've said debug will use the debug module and log* uses console.log.

As it is, I don't think this is enough for a breaking change OR for a weird API name. Maybe we can reserve doing this for the next breaking change 🤔

Or maybe it's fine to make a breaking change for this. Thoughts?

@pedroapfilho
Copy link

I don't see any issues making a breaking change for the debug API, you only use it once or twice and then remove it from the test itself.

@timdeschryver
Copy link
Member

Do we know when the next major release will be released?
I'm fine either way, but if we have some changes lined up I think it would be better to add this one to the list.

@kentcdodds
Copy link
Member

Do we know when the next major release will be released?

There's not currently any scheduled major release. And I don't think there are any other needs for a major release.

I have mixed feelings about treating this as an acceptable breaking change. I don't feel like it's necessary enough to warrant doing that. I'm not sure though.

@kentcdodds
Copy link
Member

After considering this further, I think we're too far along to make this kind of change. It would be pretty confusing to people I think 😬

@alexkrolick
Copy link
Collaborator Author

🤔 would it be a breaking change to add support for the debug env option while leaving the explicit methods as an option?

@kentcdodds
Copy link
Member

After working on an exploratory PR for this, I'm starting to think this is going to be ok. It'll be a bit of an adjustment for folks, but I think they'll figure it out.

@kentcdodds
Copy link
Member

So here's my thinking. In DTL we'll have debugDOM and logDOM which basically amount to this:

const debugDOM = (...args) => debug(prettyDOM(...args))
const logDOM = (...args) => console.log(prettyDOM(...args))

Where debug is: const debug = getDebugger('testing-lib')

In RTL, we'll make debug use debugDOM and we'll make a new method called log which calls logDOM. Thoughts?

@alexkrolick
Copy link
Collaborator Author

alexkrolick commented Aug 9, 2019

My initial thought was that this change would enable a verbose log level that doesn't require adding function calls to your tests; every render, query, and event would trigger a debug call automatically under various scopes.

You could still have the manual methods.

DEBUG=testinglib:events jest my-flaky-test
DEBUG=testinglib:queries jest my-flaky-test
DEBUG=testinglib:debug jest my-flaky-test
DEBUG=testinglib:* jest my-flaky-test

@kentcdodds
Copy link
Member

Oh, I see what you mean. So you're suggesting that we add debug calls in the DTL source code? Hmmm...

@kentcdodds
Copy link
Member

My initial thought is that if people turn that on the resulting output would be way too much to be useful.

@kentcdodds
Copy link
Member

I see what you mean. Interesting! I was thinking of something completely different. Yeah, I like this. Wouldn't require a breaking change either.

What I'm going to do is open a PR with some of my refactorings (I'm pretty happy with the resulting code), but I'll leave out my addition of debug and we can do that later.

@alexkrolick
Copy link
Collaborator Author

alexkrolick commented Aug 9, 2019

resulting output would be way too much

Maybe? You'd definitely want to debug one test at a time.

Is there an advantage to changing the debug helper otherwise? Maybe that you could keep the debugger statements in the tests for later rather than deleting them before commit?

Edit: posted at the same time, sounds good 👍

@kentcdodds
Copy link
Member

I don't think that I'll personally work on this and it looks like there's not a lot of steam behind it, so I'm going to close this for now. If someone feels strongly about it then we can bring it up again.

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

No branches or pull requests

4 participants