Skip to content

Commit e50546d

Browse files
Belco90Belco90
authored
Merge pull request #94 from testing-library/pr/no-wait-for-empty-callback
feat(no-wait-for-empty-callback ): new rule no-wait-for-empty-callback
2 parents ea3216e + db354bc commit e50546d

File tree

5 files changed

+214
-11
lines changed

5 files changed

+214
-11
lines changed

‎README.md

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
[![Tweet][tweet-badge]][tweet-url]
2424

2525
<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
26+
2627
[![All Contributors](https://img.shields.io/badge/all_contributors-13-orange.svg?style=flat-square)](#contributors-)
28+
2729
<!-- ALL-CONTRIBUTORS-BADGE:END -->
2830

2931
## Installation
@@ -132,18 +134,19 @@ To enable this configuration use the `extends` property in your
132134

133135
## Supported Rules
134136

135-
| Rule | Description | Configurations | Fixable |
136-
| --------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
137-
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
138-
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
139-
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
140-
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
141-
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
142-
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
143-
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
137+
| Rule | Description | Configurations | Fixable |
138+
| ------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
139+
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
140+
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
141+
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
142+
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
143+
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
144+
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
145+
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
144146
| [no-get-by-for-checking-element-not-present](docs/rules/no-get-by-for-checking-element-not-present.md) | Disallow the use of `getBy*` queries when checking elements are not present | | |
145-
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
146-
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
147+
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
148+
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
149+
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
147150

148151
[build-badge]: https://img.shields.io/travis/testing-library/eslint-plugin-testing-library?style=flat-square
149152
[build-url]: https://travis-ci.org/testing-library/eslint-plugin-testing-library
@@ -194,6 +197,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
194197

195198
<!-- markdownlint-enable -->
196199
<!-- prettier-ignore-end -->
200+
197201
<!-- ALL-CONTRIBUTORS-LIST:END -->
198202

199203
This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!

‎docs/rules/no-wait-for-empty-callback.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Empty callbacks inside `waitFor` and `waitForElementToBeRemoved` are not preferred (no-wait-for-empty-callback)
2+
3+
## Rule Details
4+
5+
This rule aims to ensure the correct usage of `waitFor` and `waitForElementToBeRemoved`, in the way that they're intended to be used.
6+
If an empty callback is used, these methods will just wait a tick, instead of making sure that a node was added or removed to the DOM.
7+
8+
Examples of **incorrect** code for this rule:
9+
10+
```js
11+
const foo = async () => {
12+
await waitFor(() => {});
13+
await waitFor(function() {});
14+
await waitFor(noop);
15+
16+
await waitForElementToBeRemoved(() => {});
17+
await waitForElementToBeRemoved(function() {});
18+
await waitForElementToBeRemoved(noop);
19+
};
20+
```
21+
22+
Examples of **correct** code for this rule:
23+
24+
```js
25+
const foo = async () => {
26+
await waitFor(() => {
27+
screen.getByText(/submit/i);
28+
});
29+
30+
const submit = screen.getByText(/submit/i);
31+
await waitForElementToBeRemoved(() => submit);
32+
// or
33+
await waitForElementToBeRemoved(submit);
34+
};
35+
```
36+
37+
## Further Reading
38+
39+
- [dom-testing-library v7 release](https://github.com/testing-library/dom-testing-library/releases/tag/v7.0.0)

‎lib/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const rules = {
1010
'no-dom-import': require('./rules/no-dom-import'),
1111
'no-get-by-for-checking-element-not-present': require('./rules/no-get-by-for-checking-element-not-present'),
1212
'no-manual-cleanup': require('./rules/no-manual-cleanup'),
13+
'no-wait-for-empty-callback': require('./rules/no-wait-for-empty-callback'),
1314
'prefer-explicit-assert': require('./rules/prefer-explicit-assert'),
1415
};
1516

‎lib/rules/no-wait-for-empty-callback.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
const { getDocsUrl } = require('../utils');
4+
5+
const WAIT_EXPRESSION_QUERY =
6+
'CallExpression[callee.name=/^(waitFor|waitForElementToBeRemoved)$/]';
7+
8+
module.exports = {
9+
meta: {
10+
type: 'suggestion',
11+
docs: {
12+
description:
13+
"It's not preferred to use empty callbacks in `waitFor` and `waitForElementToBeRemoved`",
14+
category: 'Best Practices',
15+
recommended: false,
16+
url: getDocsUrl('no-wait-for-empty-callback'),
17+
},
18+
messages: {
19+
noWaitForEmptyCallback:
20+
'You should verify if a node is added or removed to the DOM in `waitFor` and `waitForElementToBeRemoved`',
21+
},
22+
fixable: null,
23+
schema: [],
24+
},
25+
26+
// trimmed down implementation of https://github.com/eslint/eslint/blob/master/lib/rules/no-empty-function.js
27+
// TODO: var referencing any of previously mentioned?
28+
create: function(context) {
29+
function reportIfEmpty(node) {
30+
if (node.body.type === 'BlockStatement' && node.body.body.length === 0) {
31+
context.report({
32+
node,
33+
loc: node.body.loc.start,
34+
messageId: 'noWaitForEmptyCallback',
35+
});
36+
}
37+
}
38+
39+
function reportNoop(node) {
40+
context.report({
41+
node,
42+
loc: node.loc.start,
43+
messageId: 'noWaitForEmptyCallback',
44+
});
45+
}
46+
47+
return {
48+
[`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression`]: reportIfEmpty,
49+
[`${WAIT_EXPRESSION_QUERY} > FunctionExpression`]: reportIfEmpty,
50+
[`${WAIT_EXPRESSION_QUERY} > Identifier[name="noop"]`]: reportNoop,
51+
};
52+
},
53+
};

‎tests/lib/rules/no-wait-for-empty-callback.js

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
'use strict';
2+
3+
const rule = require('../../../lib/rules/no-wait-for-empty-callback');
4+
const RuleTester = require('eslint').RuleTester;
5+
6+
// ------------------------------------------------------------------------------
7+
// Tests
8+
// ------------------------------------------------------------------------------
9+
10+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
11+
12+
const ALL_WAIT_METHODS = ['waitFor', 'waitForElementToBeRemoved'];
13+
14+
ruleTester.run('no-wait-for-empty-callback', rule, {
15+
valid: [
16+
...ALL_WAIT_METHODS.map(m => ({
17+
code: `${m}(() => {
18+
screen.getByText(/submit/i)
19+
})`,
20+
})),
21+
...ALL_WAIT_METHODS.map(m => ({
22+
code: `${m}(function() {
23+
screen.getByText(/submit/i)
24+
})`,
25+
})),
26+
{
27+
code: `waitForElementToBeRemoved(someNode)`,
28+
},
29+
{
30+
code: `waitForElementToBeRemoved(() => someNode)`,
31+
},
32+
{
33+
code: `waitSomethingElse(() => {})`,
34+
},
35+
{
36+
code: `wait(() => {})`,
37+
},
38+
],
39+
40+
invalid: [
41+
...ALL_WAIT_METHODS.map(m => ({
42+
code: `${m}(() => {})`,
43+
errors: [
44+
{
45+
messageId: 'noWaitForEmptyCallback',
46+
},
47+
],
48+
})),
49+
...ALL_WAIT_METHODS.map(m => ({
50+
code: `${m}((a, b) => {})`,
51+
errors: [
52+
{
53+
messageId: 'noWaitForEmptyCallback',
54+
},
55+
],
56+
})),
57+
...ALL_WAIT_METHODS.map(m => ({
58+
code: `${m}(() => { /* I'm empty anyway */ })`,
59+
errors: [
60+
{
61+
messageId: 'noWaitForEmptyCallback',
62+
},
63+
],
64+
})),
65+
66+
...ALL_WAIT_METHODS.map(m => ({
67+
code: `${m}(function() {
68+
69+
})`,
70+
errors: [
71+
{
72+
messageId: 'noWaitForEmptyCallback',
73+
},
74+
],
75+
})),
76+
...ALL_WAIT_METHODS.map(m => ({
77+
code: `${m}(function(a) {
78+
79+
})`,
80+
errors: [
81+
{
82+
messageId: 'noWaitForEmptyCallback',
83+
},
84+
],
85+
})),
86+
...ALL_WAIT_METHODS.map(m => ({
87+
code: `${m}(function() {
88+
// another empty callback
89+
})`,
90+
errors: [
91+
{
92+
messageId: 'noWaitForEmptyCallback',
93+
},
94+
],
95+
})),
96+
97+
...ALL_WAIT_METHODS.map(m => ({
98+
code: `${m}(noop)`,
99+
errors: [
100+
{
101+
messageId: 'noWaitForEmptyCallback',
102+
},
103+
],
104+
})),
105+
],
106+
});

0 commit comments

Comments
 (0)