feat: added new rule prefer-in-document #95
Conversation
|
I'll keep on working on this to add fixability, improve some edge cases etc. Just didn't have enough time today. |
|
Thanks a lot for the feedback, Ben! I'll get back to you as soon as possible. I haven't written a rule before, so let me know about any best-practices, ideas etc that I might be missing. |
|
@AntonNiklasson no worries you're doing great. Appreciate your contributions |
| }, | ||
| ], | ||
| invalid: [ | ||
| ...Object.keys(queries) |
There was a problem hiding this comment.
actually thinking about this there's a couple changes I think we should make here.
- split out the get, query and find queries.
- find queries should use await
- "query" queries should be checking that
.toHaveLength(0)gets reported and fixed to.not.toBeInTheDocument()
There was a problem hiding this comment.
Yeah, I was just looking into the await case. Ah, right. Since the other ones throw instead? I'll look into it 👍
There was a problem hiding this comment.
@AntonNiklasson I thought of another couple of use cases: expect(screen.queryByText()).toBeNull() should be changed to .toBeInTheDocument() and .not.toBeNull() should be changed to .toBeInTheDocument() and conversely we should check .toBeDefined() as well.
There was a problem hiding this comment.
Yeah, good catch! I've just pushed a few changes to split the queries. I've also added some failing test cases for queryBy* and queryAllBy*. I'll get back to passing the tests in a day or two.
There was a problem hiding this comment.
I've implemented some more logic to support more matchers than just toHaveLength now. Also the negation flipping from .not.toBeNull to toBeInTheDocument. Perhaps there's more matchers I haven't thought of?
|
Please have another look @benmonro . I've made the logic a bit more generic to support more cases than just |
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 12 +2
Lines 186 218 +32
Branches 22 27 +5
=========================================
+ Hits 186 218 +32
Continue to review full report at Codecov.
|
|
@all-contributors please add @AntonNiklasson for code, tests and docs |
|
I've put up a pull request to add @AntonNiklasson! 🎉 |
benmonro
changed the title
prefer-in-document
|
🎉 This PR is included in version 3.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
I was sleeping, didn't see your comments until now 😴 Thanks a lot for merging this, Ben! I've really enjoyed working on this. Let me know if I can help with something else. |
|
Thank you!
On Mon, Nov 23, 2020 at 11:12 PM Anton Niklasson ***@***.***> wrote:
I was sleeping, didn't see your comments until now 😴 Thanks a lot for
merging this, Ben! I've really enjoyed working on this. Let me know if I
can help with something else.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#95 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADBPBB66WYP2IMPW5FFMHDSRNLULANCNFSM4TWGEWUA>
.
--
Ben Monro
Software Developer
|
What:
Create a new rule:
prefer-in-documentWhy:
Instead of asserting that a node exists in the DOM using
.toHaveLength, we could suggest to the user that they use.toBeInTheDocument(). This rule was requested in #75.Checklist: