Unnecessary presence checks on findBy queries

[nathanmmiller]

Name for new rule

no-find-by-presence

Description of the new rule

The rule should avoid things like expect(await screen.findBy*()).toBeInTheDocument(), because that's functionally the same as await screen.findBy*().

Perhaps this is a new rule (I proposed a name) or maybe it should just be part of the existing find-by rule.

Testing Library feature

await findBy

Testing Library framework(s)

All, presumaly.

What category of rule is this?

Suggests an alternate way of doing something

Optional: other category of rule

No response

Code examples

expect(await screen.findByRole("button")).toBeInTheDocument();
expect(await screen.findByText("hello")).toBeDefined();
expect(await screen.findByLabel("zoop!")).toBeTruthy();

Anything else?

I am happy to contribute this rule - but if so, it may take me a month or two to do so, as things are busy. Please let me know if you'd like me to contribute or not.

Do you want to submit a pull request to make the new rule?

Yes

[Belco90]

Hi @nathanmmiller! I think it could be a good idea to implement this rule, so give it a try if you are willing to contribute to this plugin!

We need to make sure we explain in its docs that it would be the opposite of prefer-explicit-assert, clarifying only one should be enabled, and the plugin actually recommends the explicit one. This rule could be named prefer-implicit-assert then.

[adevick]

I like this idea and have started my first open-source PR 🎉 prefer-implicit-assert #815

[adevick]

@nathanmmiller ^

[nathanmmiller]

Lovely because I honestly don't know when I was going to get to this...

…

On Mon, Sep 11, 2023 at 5:17 PM Anthony Devick ***@***.***> wrote: @nathanmmiller <https://github.com/nathanmmiller> ^ — Reply to this email directly, view it on GitHub <#743 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI6QW3Z3VWUDP5WWW5LEDRDXZ5WUDANCNFSM6AAAAAAWNX2BAY> . You are receiving this because you were mentioned.Message ID: <testing-library/eslint-plugin-testing-library/issues/743/1714517562@ github.com>

[adevick]

and the plugin actually recommends the explicit one

Why does the plugin need to recommend the explicit one? Can the community/consumers decide? Just asking because sometimes people blindly follow what the "library recommends" as the "gold standard"?

[Belco90]

@adevick We made that decision based on KCD's "Common mistakes with React Testing Library" post.

[adevick]

@adevick We made that decision based on KCD's "Common mistakes with React Testing Library" post.

That makes sense I guess that last one to me was more of his personal opinion For this reason, many people skip the assertion. This really is fine honestly, but I personally, ... but to each their own.

[nathanmmiller]

I'd actually be curious on @kentcdodds thoughts on this one now that the above was brought up.

To me, I would always do:

expect(screen.getByRole("button")).toBeInTheDocument()

over

screen.getByRole("button")

...for the same reason KCD says - it's clear that there's an intentional assertion, not just an accidental remnant of some refactor.

But, also to me,

await screen.findByRole("button")

...feels more intentional - the whole await part of it makes me feel like we're purposely asserting something ("this will show up").

That said, I don't have an extreme preference here. If this rule exists, I will turn it on, even if it's not "recommended," for precisely the reasoning above. I can see that others wouldn't because it's not "recommended" but 🤷.