new rule request: prefer-find-by

[benmonro]

as described in Kent's blog post, it would be nice to have an autofix rule that converts

await waitFor(() => {
  expect(screen.getByText("foo")).toBeInTheDocument()
}

into this:

expect (await screen.findByText("foo")).toBeInTheDocument()

[Belco90]

Hello Ben! Thanks for your suggestion. Indeed, this would be a nice addition.

Do you want to give it a try? PRs are welcome! I must admit this one could be a hard one to implement tho, as there are plenty of things you can find within waitFor callback body.

[benmonro]

I wish I could but I'm swamped at work. I noticed we have a lot of them in our code so figured I'd add the issue. :)

[gndelia]

I might be able to take a peek in this one in the incoming days 😄

[Belco90]

That would be great! Let me know if you want to discuss some details about the rule or its implementation.

[gndelia]

yeah, I have some doubts about being able to fix it in all scenarios... given the callback could potentially have tons of statements - it just seems "feasible" when the statement inside the callback is only one (like a arrow function without brackets, for instance)

Thoughts?

[Belco90]

Exactly. We can potentially find anything within callback body passed to waitFor method, so just trying to fix it for a single statement seems like the perfect approach for first implementation.

One of the latest ESLint features is the ability to suggest fixes. This looks like a good place to try it! But I'd leave it as an optional step at the end, as just implementing the rule and fixing the code for single statements is a lot of work.

[gndelia]

we could also narrow the fix if that statement inside the callback contains an expect with a get/query* method -

should we also include the deprecated methods (for instance wait) as part of the fix?

[Belco90]

That seems a good idea. And actually, let's forget about the possibility of adding suggestions and just focus on single statement within waitFor cb body.

I think it's not difficult to include deprecated wait utils when it's done for waitFor, so I'd add them too.

[gndelia]

I created a draft PR for this one.

After giving some thought and reading the best practice mentioned by kent c dodds I simplified a bit the approach:

The rule should not check for expect inside the waitFor (that might be another rule? Not using assertions inside waitFor?) but it should check for sync queries.

Basically, this scenario

await waitFor(/* some sync query */)

The example he provided is

const submitButton = await waitFor(() =>
  screen.getByRole('button', {name: /submit/i}),
)

and the proper "correct" version should be

const submitButton = await screen.findByRole('button', {name: /submit/i})

Thoughts?

as there are no assertions inside, I think this one has more chances of being automatically fixed. I will check it out (perhaps for the same PR, perhaps in a 2nd one)

[Belco90]

Oh yeah. Sorry for not leaving it clear, but here we only need to check single statement within waitFor cb body (or the returned statement), which must be a getBy query. Basically findBy is a shortcut for waitFor + getBy. I like how you worded this as waitFor + sync query as technically it could be a queryBy too.

So your approach is absolutely fine.

[Belco90]

🎉 This issue has been resolved in version 3.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀