Skip to content

Commit 88416b2

Browse files
Belco90Belco90
authored
feat: new no-unnecessary-act rule (#365)
* refactor: rename function to detect potential Testing Library func * feat(no-unnecessary-act): first approach for the rule * feat(no-unnecessary-act): first bunch of tests * test: update number of expected rules * feat(no-unnecessary-act): detect error from returns * refactor: extract util to determine if a function is empty * feat(no-unnecessary-act): detect error for empty callbacks * refactor(no-unnecessary-act): function to find non-testing-library calls * refactor(no-unnecessary-act): update docs recommended config * fix: check if node is coming from Testing Library correctly isNodeComingFromTestingLibrary wasn't checking if the imported declaration name matches some Testing Library valid module * feat: save react-dom/test-utils import in detection mechanism * refactor: extract function for finding import specifier * refactor(no-unnecessary-act): detect act from React DOM test utils * fix: remove duplicated isVariableDeclaration declaration * feat(no-unnecessary-act): detect edge and mixed cases * docs(no-unnecessary-act): add rule details * docs(no-unnecessary-act): include rule in README * refactor: rename findImportedTestingLibraryUtilSpecifier in helpers * docs(no-unnecessary-act): simplify examples * refactor: remove unnecessary comment * fix(no-unnecessary-act): cover more trees searching for statements * test(no-unnecessary-act): extra cases for thenable promises Closes #259 Co-authored-by: @alessbell
1 parent 816df6e commit 88416b2

File tree

8 files changed

+1371
-82
lines changed

8 files changed

+1371
-82
lines changed

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,16 @@ To enable this configuration use the `extends` property in your
193193
| [testing-library/no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
194194
| [testing-library/no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
195195
| [testing-library/no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | |
196+
| [testing-library/no-unnecessary-act](docs/rules/no-unnecessary-act.md) | Disallow wrapping Testing Library utils or empty callbacks in `act` | | |
196197
| [testing-library/no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
197198
| [testing-library/no-wait-for-multiple-assertions](docs/rules/no-wait-for-multiple-assertions.md) | Disallow the use of multiple expect inside `waitFor` | | |
198199
| [testing-library/no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects inside `waitFor` | | |
199200
| [testing-library/no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
200201
| [testing-library/prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
201202
| [testing-library/prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
202203
| [testing-library/prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
203-
| [testing-library/prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` library instead of `fireEvent` for simulating user interaction | | |
204204
| [testing-library/prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
205+
| [testing-library/prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` library instead of `fireEvent` for simulating user interaction | | |
205206
| [testing-library/prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] |
206207
| [testing-library/render-result-naming-convention](docs/rules/render-result-naming-convention.md) | Enforce a valid naming for return value from `render` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
207208

‎docs/rules/no-unnecessary-act.md‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Disallow wrapping Testing Library utils or empty callbacks in `act` (`testing-library/no-unnecessary-act`)
2+
3+
> ⚠️ The `act` method is only available on the following Testing Library packages:
4+
>
5+
> - `@testing-library/react` (supported by this plugin)
6+
> - `@testing-library/preact` (not supported yet by this plugin)
7+
> - `@testing-library/svelte` (not supported yet by this plugin)
8+
9+
## Rule Details
10+
11+
This rule aims to avoid the usage of `act` to wrap Testing Library utils just to silence "not wrapped in act(...)" warnings.
12+
13+
All Testing Library utils are already wrapped in `act`. Most of the time, if you're seeing an `act` warning, it's not just something to be silenced, but it's actually telling you that something unexpected is happening in your test.
14+
15+
Additionally, wrapping empty callbacks in `act` is also an incorrect way of silencing "not wrapped in act(...)" warnings.
16+
17+
Code violations reported by this rule will pinpoint those unnecessary `act`, helping to understand when `act` actually is necessary.
18+
19+
Example of **incorrect** code for this rule:
20+
21+
```js
22+
// ❌ wrapping things related to Testing Library in `act` is incorrect
23+
import {
24+
act,
25+
render,
26+
screen,
27+
waitFor,
28+
fireEvent,
29+
} from '@testing-library/react';
30+
// ^ act imported from 'react-dom/test-utils' will be reported too
31+
import userEvent from '@testing-library/user-event';
32+
33+
// ...
34+
35+
act(() => {
36+
render(<Example />);
37+
});
38+
39+
await act(async () => waitFor(() => {}));
40+
41+
act(() => screen.getByRole('button'));
42+
43+
act(() => {
44+
fireEvent.click(element);
45+
});
46+
47+
act(() => {
48+
userEvent.click(element);
49+
});
50+
```
51+
52+
```js
53+
// ❌ wrapping empty callbacks in `act` is incorrect
54+
import { act } from '@testing-library/react';
55+
// ^ act imported from 'react-dom/test-utils' will be reported too
56+
import userEvent from '@testing-library/user-event';
57+
58+
// ...
59+
60+
act(() => {});
61+
62+
await act(async () => {});
63+
```
64+
65+
Examples of **correct** code for this rule:
66+
67+
```js
68+
// ✅ wrapping things not related to Testing Library in `act` is correct
69+
import { act } from '@testing-library/react';
70+
import { stuffThatDoesNotUseRTL } from 'somwhere-else';
71+
72+
// ...
73+
74+
act(() => {
75+
stuffThatDoesNotUseRTL();
76+
});
77+
```
78+
79+
```js
80+
// ✅ wrapping both things related and not related to Testing Library in `act` is correct
81+
import { act, screen } from '@testing-library/react';
82+
import { stuffThatDoesNotUseRTL } from 'somwhere-else';
83+
84+
await act(async () => {
85+
await screen.findByRole('button');
86+
stuffThatDoesNotUseRTL();
87+
});
88+
```
89+
90+
## Further Reading
91+
92+
- [Inspiration for this rule](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#wrapping-things-in-act-unnecessarily)
93+
- [Fix the "not wrapped in act(...)" warning](https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning)
94+
- [About React Testing Library `act`](https://testing-library.com/docs/react-testing-library/api/#act)

0 commit comments

Comments
 (0)