prefer-find-by autofix doesn't respect extra code
Belco90 opened this issue ยท 10 comments
Plugin version
v5.3.1
ESLint version
v8.14.0
Node.js version
v16.3.0
npm/yarn version
npm v8.9.0
Operating system
macOS v12.3.1
Bug description
The autofix of the rule prefer-find-by doesn't work correctly when waitFor options are provided, or there is an assertion involved.
Steps to reproduce
- Providing waitFor options (โ
fixed):
const button = await waitFor(() => screen.getByText('Count is: 0'), { timeout: 100, })
- There is an assertion involved
await waitFor(() => expect( screen.getByRole('button', { name: 'Count is: 0' }), ).toBeInTheDocument(), )
Error output/screenshots
These are fixed as:
- Providing waitFor options:
const button = await screen.findByText('Count is: 0')
- There is an assertion involved
await screen.findByRole('button', { name: 'Count is: 0' })
ESLint configuration
N/A
Rule(s) affected
prefer-find-by
Anything else?
They should be fixed as:
- Providing waitFor options:
const button = await screen.findByText('Count is: 0', { timeout: 100 })
- There is an assertion involved
expect( await screen.findByRole('button', { name: 'Count is: 0' }), ).toBeInTheDocument(),
Do you want to submit a pull request to fix this bug?
Yes
I started looking into this, and... I think that this issue's options part might be unnecessary ๐
Yes, the waitFor can be passed options that are in the shape of
options?: {
container?: HTMLElement
timeout?: number
interval?: number
onTimeout?: (error: Error) => Error
mutationObserverOptions?: MutationObserverInit
}However, the different queries (ByText, ByLabelText, etc) can also be passed options, but they are of totally different shape (this varies by the query, but most common case is):
options?: {
exact?: boolean = true,
normalizer?: NormalizerFn,
}Our unit tests for prefer-find-by already test that query options are being passed correctly after auto-fixing
That's right, but this is referring to findBy* queries, which is a wrapper over waitFor util + getBy* queries. This is mentioned in the official docs (I should have linked this in the ticket description, my bad):
findBy methods are a combination of getBy queries and waitFor. They accept the waitFor options as the last argument (e.g. await screen.findByText('text', queryOptions, waitForOptions)).
So based on this, we should keep the options found in the waitFor util, and pass them as the last argument to findBy* queries. You can reproduce this with the examples I left in the description: the options disappear.
Additionally, this issue describes another issue when the waitFor is wrapping an assertion. Perhaps we can split these two bugs in separate issues.
Aaah, thank you for clarifying @Belco90! I learned something new today
Okay, #679 fixes the part/scenario 1 of this issue. I'll continue working on the second part too, so I'm assigning this to myself.
Reopening since only the first half is done (GitHub automation closed it, my bad).