Closed mikew closed 9 months ago
mikew
commented
9 months ago I think I will just disable this rule, as it stands it feels pretty inconsistent, and the developer will most likely start with act, which doesn't break anything, and then be told to remove it, which might produce a warning. And I don't want to litter our code with eslint-disable-next-line comments.
Belco90
commented
9 months ago Hi @mikew! I'm afraid the errors reported are not false positives but legit potential issues.
You can find a more detailed explanation in the links provided in the rule's doc. Basically, the problem is that Testing Library utils are already wrapped in act so needing to wrap their usage in act again is a sign of something is incorrect in your test. If you remove the act, you get warnings not because you actually need it, but because there is an async action your test is not aware of. You must await for something changing in your UI (e.g., a new element appears, the value of an existing element changes, an existing element disappears) after you perform the actions.
Based on the provided code snippet, it should be something like:
screen.getByTestId('ExampleComponent--incrementByInput').focus()
// Assuming you are using user-event v14, awaiting the userEvent.type is fine. Otherwise, it's not.
await userEvent.type(
screen.getByTestId('ExampleComponent--incrementByInput'),
'3',
// This seems like an issue with user-event. Focusing an input should
// select all the text.
{
initialSelectionStart: 0,
initialSelectionEnd: 1,
},
)
// I'm assuming the input has a new value now, so I'll wait for that
await waitFor(() => expect(screen.getByTestId('ExampleComponent--incrementByInput')).toHaveValue('3'))This is definitely not a bug. However, if you are not in a position where you can fix the reported issues, feel free to just disable the rule. I encourage you to find what async behavior is changing the UI after those actions and wait for them instead, tho.
mikew
commented
9 months ago I appreciate the thorough response! This is a very basic test that assets something is changed through very synchronous redux, so probably little more than any context + updating interaction. I've gone ahead and disabled the rule, and this just helped me solidify my views that my team should keep using playwright / cyprus.
Have you read the Troubleshooting section?
Yes
Plugin version
v5.11.1
ESLint version
v8.50.0
Node.js version
v18.16.1
package manager and version
npm v9.5.1
Operating system
Linux
Bug description
I'm upgrading dependencies and am getting a lot of churn from testing-library. It's now complaining that the
actbelow is unnecessary, but without it we get a giant warning:Steps to reproduce
Error output/screenshots
No response
ESLint configuration
Rule(s) affected
testing-library/no-unnecessary-act
Anything else?
The docs for focus / blur generate a warning for testing-library/prefer-screen-queries, also a TypeScript error because that function seems to want 2 arguments.
My reference to churn is due to the fact that this is happening in a starter kit for the company I work at. Every few months I go and upgrade the dependencies. The few tests that are in the repo are usually effected, even though there's no changes to the UI code. The last push I did apparently added
act, due to warnings. Now testing-library is telling me to remove them again.Do you want to submit a pull request to fix this bug?
Yes