-
Notifications
You must be signed in to change notification settings - Fork 150
Avoid using throwing queries with expect: no-expect-get-by-query #21
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
Comments
Nice, this is recommended in testing library doc too so it would definitely be a good addition. Anyway, there are couple of examples in your invalid cases I don't understand:
So I think |
Yeah, true. I updated the comment! |
Could, everything lgtm now! Do you want to take care of this one? I'll try to build something for #20 meanwhile. |
Sure! 🙂 |
FYI: my team and I started recently to also implement some eslint rules for RTL. In fact we have one that basically solves this problem https://www.npmjs.com/package/eslint-plugin-testing-library-react @Belco90 would you be open if we contribute to your list of rules, so that we deprecate our package? WDYT? |
Hey @emmenko! Absolutely, I'll be more than happy if you contribute to this plugin with that rule and even collaborate together in the future with more rules. The only thing is I see your plugin is focused in just react, but the rule should be generic enough to work also with dom, vue and angular for this plugin, which I think it already is but we would have to double check. I'm not sure how to proceed from here, maybe @thomlom can tell us how far he got the rule so we can get it working together with your rule implementation? |
Sure thing. Let me know if we have the green light to make a PR with our rule. |
Of course! There is no better way to review the rule being included here for sure. |
I opened a PR with the new rule. |
Uh oh!
There was an error while loading. Please reload this page.
Problem
According to the cheatsheet,
getBy
,getAllBy
are queries that throw an error if there are no matches. Thus, it doesn't make sense to useexpect
with a query that throws:I don't know if that's a thing but I'm more used to use:
Solution
The rule (let's call it
no-expect-throwing-query
but I'm not sure of that name) would preventgetBy
andgetAllBy
to be used withexpect
as they already throw an error.Valid cases:
Invalid cases:
The text was updated successfully, but these errors were encountered: