eslint-plugin-testing-library icon indicating copy to clipboard operation
eslint-plugin-testing-library copied to clipboard

Incorrect prefer-find-by autofix when using within inside waitFor

Open atsikov opened this issue 3 years ago • 3 comments

Plugin version

v5.0.3

ESLint version

v8.7.0

Node.js version

v16.13.1

npm/yarn version

pnpm v5.18.0

Operating system

macOS Big Sur 11.6.2

Bug description

await waitFor blocks with within and chained getBy... queries are not correctly autofixed

Steps to reproduce

The following snippet raises a prefer-find-by error

const btnProceed = await waitFor(() =>
  within(screen.getByRole('dialog', { name: 'Confirmation' })).getByRole(
    'button',
    { name: 'Proceed' },
  ),
)

Rule autofixer produces the following code, losing a query for the button itself

const btnProceed = await screen.findByRole('dialog', { name: 'Confirmation' })

Error output/screenshots

No response

ESLint configuration

'testing-library/await-async-query': ['warn'],
'testing-library/await-async-utils': ['warn'],
'testing-library/no-await-sync-query': ['warn'],
'testing-library/no-debugging-utils': ['warn'],
'testing-library/no-dom-import': ['warn'],
'testing-library/no-manual-cleanup': ['warn'],
'testing-library/no-wait-for-empty-callback': ['warn'],
'testing-library/prefer-explicit-assert': ['warn', { includeFindQueries: false }],
'testing-library/prefer-find-by': ['warn'],
'testing-library/prefer-presence-queries': ['warn'],
'testing-library/prefer-screen-queries': ['warn'],
'testing-library/prefer-wait-for': ['warn'],

Rule(s) affected

testing-library/prefer-find-by

Anything else?

It should not be a difficult thing to fix, and I could submit a PR, however I'm not sure what's a better way to approach it. Generally, there could be 3 different solutions:

  1. Do not trigger prefer-find-by for waitFor assertions like within(screen.getBy...()).getBy...()
  2. Autofix as await within(await screen.findBy...(...)).findBy...(...)
  3. Trigger an error but do not autofix such cases

Do you want to submit a pull request to fix this bug?

Yes, but need help

atsikov avatar Jan 19 '22 10:01 atsikov

Hey @atsikov, thanks for reporting!

Good catch. To be honest, not really sure what would be the best approach from the options you suggested. I'd go for number 2, but perhaps as a suggestion rather than as a fix. If not, then I'd go for number 3 so we report it at least. What do you think?

Belco90 avatar Jan 19 '22 15:01 Belco90

Actually, my case turned out to be non-fixable.

We have a few dialogs opened one by one, with the same name, but different set of elements. So I actually awaited for the next dialog and a button inside it. So with waitFor, I was waiting for both queries to return elements at the same time. Such findBy... code will not work for this case, as it will find already opened dialog right away, which will not contain "Proceed" button

await within(
  await screen.findByRole('dialog', { name: 'Confirmation' })
).findByRole('button', { name: 'Proceed' })

At this point, I'm leaning towards not providing even a suggestion (or not reporting at all), as such suggestion might change the expected behaviour.

atsikov avatar Jan 19 '22 16:01 atsikov

Interesting case! I guess we can avoid triggering an error if a query chained to within is found inside a waitFor (option 1), and describe this situation in the rule's docs.

Belco90 avatar Jan 19 '22 16:01 Belco90