-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ESLint: Add and enable eslint-plugin-jest-dom
#44983
Conversation
const invisibleTooltipTrigger = screen.getByText( | ||
invisibleTooltipTriggerContent | ||
); | ||
// The invisible tooltip should not render. | ||
expect( tooltips ).toHaveLength( 1 ); | ||
// The base tooltip should render only; invisible tooltip should not render. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit — I'm not sure I understand this inline comment!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason in the original test, the "WordPress.org - Invisible" text is referred to as "invisible tooltip". I've just kept that reference. But I'm happy to improve it if you have an idea why we were referring to it as "invisible tooltip" in the first place!
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that if you try using element.textContent or element.getAttribute() in your test, you'll get an error.
This is working well for me! My IDE is even auto-fixing those linter errors when saving.
🚀
Thanks for the feedback here and in the related PR, everyone! I'm shipping this one, and I'm happy to disable some rules in the future if we end up considering them too restrictive. However, I do believe they are pretty straightforward and I don't see that likely to happen. |
Nice work, @tyxla 👍 |
What?
This PR introduces the
eslint-plugin-jest-dom
ESLint plugin and enables it for all non-native test files.Why?
It aims to help maintain following the best practices when writing tests with
jest-dom
. There's been a discussion around how restrictive those are, and I believe they ensure best practices while being not-so-restrictive, but I'm happy to take feedback.How?
We're enabling the plugin and all its recommended rules for all test files, and excluding all native tests because there's a separate library for it, and we're not using it just yet.
We're also fixing a false positive in a test - the only remaining one that wasn't following the recommended rules.
Testing Instructions
element.textContent
orelement.getAttribute()
in your test, you'll get an error.