Skip to content

Commit 7a9af37

Browse files
gndeliagndelia
authored
feat: add prefer-find-by rule (#144)
Closes #127
1 parent d4a924d commit 7a9af37

File tree

7 files changed

+286
-15
lines changed

7 files changed

+286
-15
lines changed

‎README.md

+19-15
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-20-orange.svg?style=flat-square)](#contributors-)
28+
2729
<!-- ALL-CONTRIBUTORS-BADGE:END -->
2830

2931
## Installation
@@ -131,21 +133,22 @@ To enable this configuration use the `extends` property in your
131133

132134
## Supported Rules
133135

134-
| Rule | Description | Configurations | Fixable |
135-
| ---------------------------------------------------------------------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
136-
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
137-
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
138-
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
139-
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
140-
| [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][] | |
141-
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
142-
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
143-
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
144-
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
145-
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
146-
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
147-
| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | | |
148-
| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] |
136+
| Rule | Description | Configurations | Fixable |
137+
| ---------------------------------------------------------------------- | -------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
138+
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
139+
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
140+
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
141+
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
142+
| [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][] | |
143+
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
144+
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
145+
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
146+
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
147+
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
148+
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
149+
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
150+
| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | | |
151+
| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] |
149152

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

206209
<!-- markdownlint-enable -->
207210
<!-- prettier-ignore-end -->
211+
208212
<!-- ALL-CONTRIBUTORS-LIST:END -->
209213

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

‎docs/rules/prefer-find-by.md

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries (prefer-find-by)
2+
3+
findBy* queries are a simple combination of getBy* queries and waitFor. The findBy\* queries accept the waitFor options as the last argument. (i.e. screen.findByText('text', queryOptions, waitForOptions))
4+
5+
## Rule details
6+
7+
This rule aims to use `findBy*` or `findAllBy*` queries to wait for elements, rather than using `waitFor`, or the deprecated methods `waitForElement` and `wait`.
8+
This rules analyzes those cases where `waitFor` is used with just one query method, in the form of an arrow function with only one statement (that is, without a block of statements). Given the callback could be more complex, this rule does not consider function callbacks or arrow functions with blocks of code
9+
10+
Examples of **incorrect** code for this rule
11+
12+
```js
13+
// arrow functions with one statement, using screen and any sync query method
14+
const submitButton = await waitFor(() =>
15+
screen.getByRole('button', { name: /submit/i })
16+
);
17+
const submitButton = await waitFor(() =>
18+
screen.getAllByTestId('button', { name: /submit/i })
19+
);
20+
21+
// arrow functions with one statement, calling any sync query method
22+
const submitButton = await waitFor(() =>
23+
queryByLabel('button', { name: /submit/i })
24+
);
25+
26+
const submitButton = await waitFor(() =>
27+
queryAllByText('button', { name: /submit/i })
28+
);
29+
```
30+
31+
Examples of **correct** code for this rule:
32+
33+
```js
34+
// using findBy* methods
35+
const submitButton = await findByText('foo');
36+
const submitButton = await screen.findAllByRole('table');
37+
38+
// using waitForElementToBeRemoved
39+
await waitForElementToBeRemoved(() => screen.findAllByRole('button'));
40+
await waitForElementToBeRemoved(() => queryAllByLabel('my label'));
41+
await waitForElementToBeRemoved(document.querySelector('foo'));
42+
43+
// using waitFor with a function
44+
await waitFor(function() {
45+
foo();
46+
return getByText('name');
47+
});
48+
49+
// passing a reference of a function
50+
function myCustomFunction() {
51+
foo();
52+
return getByText('name');
53+
}
54+
await waitFor(myCustomFunction);
55+
56+
// using waitFor with an arrow function with a code block
57+
await waitFor(() => {
58+
baz();
59+
return queryAllByText('foo');
60+
});
61+
62+
// using a custom arrow function
63+
await waitFor(() => myCustomFunction());
64+
65+
// using expects inside waitFor
66+
await waitFor(() => expect(screen.getByText('bar')).toBeDisabled());
67+
await waitFor(() => expect(getAllByText('bar')).toBeDisabled());
68+
```
69+
70+
## When Not To Use It
71+
72+
- Not encouraging use of findBy shortcut from testing library best practices
73+
74+
## Further Reading
75+
76+
- Documentation for [findBy\* queries](https://testing-library.com/docs/dom-testing-library/api-queries#findby)
77+
78+
- Common mistakes with RTL, by Kent C. Dodds: [Using waitFor to wait for elements that can be queried with find\*](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#using-waitfor-to-wait-for-elements-that-can-be-queried-with-find)

‎lib/index.ts

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import preferExplicitAssert from './rules/prefer-explicit-assert';
1111
import preferPresenceQueries from './rules/prefer-presence-queries';
1212
import preferScreenQueries from './rules/prefer-screen-queries';
1313
import preferWaitFor from './rules/prefer-wait-for';
14+
import preferFindBy from './rules/prefer-find-by';
1415

1516
const rules = {
1617
'await-async-query': awaitAsyncQuery,
@@ -23,6 +24,7 @@ const rules = {
2324
'no-manual-cleanup': noManualCleanup,
2425
'no-wait-for-empty-callback': noWaitForEmptyCallback,
2526
'prefer-explicit-assert': preferExplicitAssert,
27+
'prefer-find-by': preferFindBy,
2628
'prefer-presence-queries': preferPresenceQueries,
2729
'prefer-screen-queries': preferScreenQueries,
2830
'prefer-wait-for': preferWaitFor,
@@ -32,6 +34,7 @@ const recommendedRules = {
3234
'testing-library/await-async-query': 'error',
3335
'testing-library/await-async-utils': 'error',
3436
'testing-library/no-await-sync-query': 'error',
37+
'testing-library/prefer-find-by': 'error',
3538
};
3639

3740
export = {

‎lib/node-utils.ts

+4
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,7 @@ export function hasThenProperty(node: TSESTree.Node) {
102102
node.property.name === 'then'
103103
);
104104
}
105+
106+
export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
107+
return node.type === 'ArrowFunctionExpression'
108+
}

‎lib/rules/prefer-find-by.ts

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import {
3+
isIdentifier,
4+
isCallExpression,
5+
isMemberExpression,
6+
isArrowFunctionExpression,
7+
} from '../node-utils';
8+
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';
9+
10+
export const RULE_NAME = 'prefer-find-by';
11+
12+
type Options = [];
13+
export type MessageIds = 'preferFindBy';
14+
15+
export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait']
16+
17+
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
18+
name: RULE_NAME,
19+
meta: {
20+
type: 'suggestion',
21+
docs: {
22+
description: 'Suggest using find* instead of waitFor to wait for elements',
23+
category: 'Best Practices',
24+
recommended: 'warn',
25+
},
26+
messages: {
27+
preferFindBy: 'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}'
28+
},
29+
fixable: null,
30+
schema: []
31+
},
32+
defaultOptions: [],
33+
34+
create(context) {
35+
36+
function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, fullQuery }: { queryVariant: string, queryMethod: string, fullQuery: string}) {
37+
context.report({
38+
node,
39+
messageId: "preferFindBy",
40+
data: { queryVariant, queryMethod, fullQuery },
41+
});
42+
}
43+
44+
const sourceCode = context.getSourceCode();
45+
46+
return {
47+
'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) {
48+
if (!isIdentifier(node.callee) || !WAIT_METHODS.includes(node.callee.name)) {
49+
return
50+
}
51+
// ensure the only argument is an arrow function expression - if the arrow function is a block
52+
// we skip it
53+
const argument = node.arguments[0]
54+
if (!isArrowFunctionExpression(argument)) {
55+
return
56+
}
57+
if (!isCallExpression(argument.body)) {
58+
return
59+
}
60+
// ensure here it's one of the sync methods that we are calling
61+
if (isMemberExpression(argument.body.callee) && isIdentifier(argument.body.callee.property) && isIdentifier(argument.body.callee.object) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)) {
62+
// shape of () => screen.getByText
63+
const queryMethod = argument.body.callee.property.name
64+
reportInvalidUsage(node, {
65+
queryMethod: queryMethod.split('By')[1],
66+
queryVariant: getFindByQueryVariant(queryMethod),
67+
fullQuery: sourceCode.getText(node)
68+
})
69+
return
70+
}
71+
if (isIdentifier(argument.body.callee) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)) {
72+
// shape of () => getByText
73+
const queryMethod = argument.body.callee.name
74+
reportInvalidUsage(node, {
75+
queryMethod: queryMethod.split('By')[1],
76+
queryVariant: getFindByQueryVariant(queryMethod),
77+
fullQuery: sourceCode.getText(node)
78+
})
79+
return
80+
}
81+
}
82+
}
83+
}
84+
})
85+
86+
function getFindByQueryVariant(queryMethod: string) {
87+
return queryMethod.includes('All') ? 'findAllBy' : 'findBy'
88+
}

‎tests/__snapshots__/index.test.ts.snap

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Object {
1414
"error",
1515
"angular",
1616
],
17+
"testing-library/prefer-find-by": "error",
1718
},
1819
}
1920
`;
@@ -32,6 +33,7 @@ Object {
3233
"error",
3334
"react",
3435
],
36+
"testing-library/prefer-find-by": "error",
3537
},
3638
}
3739
`;
@@ -45,6 +47,7 @@ Object {
4547
"testing-library/await-async-query": "error",
4648
"testing-library/await-async-utils": "error",
4749
"testing-library/no-await-sync-query": "error",
50+
"testing-library/prefer-find-by": "error",
4851
},
4952
}
5053
`;
@@ -64,6 +67,7 @@ Object {
6467
"error",
6568
"vue",
6669
],
70+
"testing-library/prefer-find-by": "error",
6771
},
6872
}
6973
`;

‎tests/lib/rules/prefer-find-by.test.ts

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { InvalidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
2+
import { createRuleTester } from '../test-utils';
3+
import { ASYNC_QUERIES_COMBINATIONS, SYNC_QUERIES_COMBINATIONS } from '../../../lib/utils';
4+
import rule, { WAIT_METHODS, RULE_NAME } from '../../../lib/rules/prefer-find-by';
5+
6+
const ruleTester = createRuleTester({
7+
ecmaFeatures: {
8+
jsx: true,
9+
},
10+
});
11+
12+
ruleTester.run(RULE_NAME, rule, {
13+
valid: [
14+
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
15+
code: `const submitButton = await ${queryMethod}('foo')`
16+
})),
17+
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
18+
code: `const submitButton = await screen.${queryMethod}('foo')`
19+
})),
20+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
21+
code: `await waitForElementToBeRemoved(() => ${queryMethod}(baz))`
22+
})),
23+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
24+
code: `await waitFor(function() {
25+
return ${queryMethod}('baz', { name: 'foo' })
26+
})`
27+
})),
28+
{
29+
code: `await waitFor(() => myCustomFunction())`
30+
},
31+
{
32+
code: `await waitFor(customFunctionReference)`
33+
},
34+
{
35+
code: `await waitForElementToBeRemoved(document.querySelector('foo'))`
36+
},
37+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
38+
code: `
39+
await waitFor(() => {
40+
foo()
41+
return ${queryMethod}()
42+
})
43+
`
44+
})),
45+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
46+
code: `
47+
await waitFor(() => expect(screen.${queryMethod}('baz')).toBeDisabled());
48+
`
49+
})),
50+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
51+
code: `
52+
await waitFor(() => expect(${queryMethod}('baz')).toBeInTheDocument());
53+
`
54+
}))
55+
],
56+
invalid: [
57+
// using reduce + concat 'cause flatMap is not available in node10.x
58+
...WAIT_METHODS.reduce((acc: InvalidTestCase<'preferFindBy', []>[], waitMethod) => acc
59+
.concat(
60+
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
61+
code: `
62+
const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))
63+
`,
64+
errors: [{
65+
messageId: 'preferFindBy',
66+
data: {
67+
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
68+
queryMethod: queryMethod.split('By')[1],
69+
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
70+
}
71+
}]
72+
}))
73+
).concat(
74+
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
75+
code: `
76+
const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))
77+
`,
78+
errors: [{
79+
messageId: 'preferFindBy',
80+
data: {
81+
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
82+
queryMethod: queryMethod.split('By')[1],
83+
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
84+
}
85+
}]
86+
}))
87+
),
88+
[])
89+
],
90+
})

0 commit comments

Comments
 (0)