Skip to content

Commit 8f49686

Browse files
gndeliagndelia
committed
feat: add prefer-find-by rule
1 parent b3e46d8 commit 8f49686

File tree

5 files changed

+248
-0
lines changed

5 files changed

+248
-0
lines changed

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

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Use `find*` query methods to wait for elements instead of waitFor (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.getAllTestId('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+
66+
## When Not To Use It
67+
68+
- Not encouraging use of findBy shortcut from testing library best practices
69+
70+
## Further Reading
71+
72+
- Documentation for [findBy\* queries](https://testing-library.com/docs/dom-testing-library/api-queries#findby)
73+
74+
- 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

+2
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,

‎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+
// TODO check if this should be under utils.ts - there are some async utils
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: 'error',
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/lib/rules/prefer-find-by.test.ts

+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
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: string) => ({
15+
code: `const submitButton = await ${queryMethod}('foo')`
16+
})),
17+
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
18+
code: `const submitButton = await screen.${queryMethod}('foo')`
19+
})),
20+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
21+
code: `await waitForElementToBeRemoved(() => ${queryMethod}(baz))`
22+
})),
23+
...SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
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: string) => ({
38+
code: `
39+
await waitFor(() => {
40+
foo()
41+
return ${queryMethod}()
42+
})
43+
`
44+
})),
45+
],
46+
invalid: [
47+
// using reduce + concat 'cause flatMap is not available in node10.x
48+
...WAIT_METHODS.reduce((acc: InvalidTestCase<'preferFindBy', []>[], waitMethod) => acc
49+
.concat(
50+
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
51+
code: `
52+
const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))
53+
`,
54+
errors: [{
55+
messageId: 'preferFindBy',
56+
data: {
57+
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
58+
queryMethod: queryMethod.split('By')[1],
59+
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
60+
}
61+
}]
62+
}))
63+
).concat(
64+
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
65+
code: `
66+
const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))
67+
`,
68+
errors: [{
69+
messageId: 'preferFindBy',
70+
data: {
71+
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
72+
queryMethod: queryMethod.split('By')[1],
73+
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
74+
}
75+
}]
76+
}))
77+
),
78+
[])
79+
],
80+
})

0 commit comments

Comments
 (0)