Skip to content

Commit

Permalink
feat: add prefer-find-by rule
Browse files Browse the repository at this point in the history
  • Loading branch information
gndelia committed May 29, 2020
1 parent d4a924d commit 38a67f7
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 0 deletions.
74 changes: 74 additions & 0 deletions docs/rules/prefer-find-by.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Use `find*` query methods to wait for elements instead of waitFor (prefer-find-by)

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))

## Rule details

This rule aims to use `findBy*` or `findAllBy*` queries to wait for elements, rather than using `waitFor`, or the deprecated methods `waitForElement` and `wait`.
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

Examples of **incorrect** code for this rule

```js
// arrow functions with one statement, using screen and any sync query method
const submitButton = await waitFor(() =>
screen.getByRole('button', { name: /submit/i })
);
const submitButton = await waitFor(() =>
screen.getAllTestId('button', { name: /submit/i })
);

// arrow functions with one statement, calling any sync query method
const submitButton = await waitFor(() =>
queryByLabel('button', { name: /submit/i })
);

const submitButton = await waitFor(() =>
queryAllByText('button', { name: /submit/i })
);
```

Examples of **correct** code for this rule:

```js
// using findBy* methods
const submitButton = await findByText('foo');
const submitButton = await screen.findAllByRole('table');

// using waitForElementToBeRemoved
await waitForElementToBeRemoved(() => screen.findAllByRole('button'));
await waitForElementToBeRemoved(() => queryAllByLabel('my label'));
await waitForElementToBeRemoved(document.querySelector('foo'));

// using waitFor with a function
await waitFor(function() {
foo();
return getByText('name');
});

// passing a reference of a function
function myCustomFunction() {
foo();
return getByText('name');
}
await waitFor(myCustomFunction);

// using waitFor with an arrow function with a code block
await waitFor(() => {
baz();
return queryAllByText('foo');
});

// using a custom arrow function
await waitFor(() => myCustomFunction());
```

## When Not To Use It

- Not encouraging use of findBy shortcut from testing library best practices

## Further Reading

- Documentation for [findBy\* queries](https://testing-library.com/docs/dom-testing-library/api-queries#findby)

- 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)
2 changes: 2 additions & 0 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import preferExplicitAssert from './rules/prefer-explicit-assert';
import preferPresenceQueries from './rules/prefer-presence-queries';
import preferScreenQueries from './rules/prefer-screen-queries';
import preferWaitFor from './rules/prefer-wait-for';
import preferFindBy from './rules/prefer-find-by';

const rules = {
'await-async-query': awaitAsyncQuery,
Expand All @@ -23,6 +24,7 @@ const rules = {
'no-manual-cleanup': noManualCleanup,
'no-wait-for-empty-callback': noWaitForEmptyCallback,
'prefer-explicit-assert': preferExplicitAssert,
'prefer-find-by': preferFindBy,
'prefer-presence-queries': preferPresenceQueries,
'prefer-screen-queries': preferScreenQueries,
'prefer-wait-for': preferWaitFor,
Expand Down
4 changes: 4 additions & 0 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,7 @@ export function hasThenProperty(node: TSESTree.Node) {
node.property.name === 'then'
);
}

export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
return node.type === 'ArrowFunctionExpression'
}
88 changes: 88 additions & 0 deletions lib/rules/prefer-find-by.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import {
isIdentifier,
isCallExpression,
isMemberExpression,
isArrowFunctionExpression,
} from '../node-utils';
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';

export const RULE_NAME = 'prefer-find-by';

type Options = [];
export type MessageIds = 'preferFindBy';
// TODO check if this should be under utils.ts - there are some async utils
export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait']

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'suggestion',
docs: {
description: 'Suggest using find* instead of waitFor to wait for elements',
category: 'Best Practices',
recommended: 'warn',
},
messages: {
preferFindBy: 'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}'
},
fixable: null,
schema: []
},
defaultOptions: [],

create(context) {

function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, fullQuery }: { queryVariant: string, queryMethod: string, fullQuery: string}) {
context.report({
node,
messageId: "preferFindBy",
data: { queryVariant, queryMethod, fullQuery },
});
}

const sourceCode = context.getSourceCode();

return {
'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) {
if (!isIdentifier(node.callee) || !WAIT_METHODS.includes(node.callee.name)) {
return
}
// ensure the only argument is an arrow function expression - if the arrow function is a block
// we skip it
const argument = node.arguments[0]
if (!isArrowFunctionExpression(argument)) {
return
}
if (!isCallExpression(argument.body)) {
return
}
// ensure here it's one of the sync methods that we are calling
if (isMemberExpression(argument.body.callee) && isIdentifier(argument.body.callee.property) && isIdentifier(argument.body.callee.object) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)) {
// shape of () => screen.getByText
const queryMethod = argument.body.callee.property.name
reportInvalidUsage(node, {
queryMethod: queryMethod.split('By')[1],
queryVariant: getFindByQueryVariant(queryMethod),
fullQuery: sourceCode.getText(node)
})
return
}
if (isIdentifier(argument.body.callee) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)) {
// shape of () => getByText
const queryMethod = argument.body.callee.name
reportInvalidUsage(node, {
queryMethod: queryMethod.split('By')[1],
queryVariant: getFindByQueryVariant(queryMethod),
fullQuery: sourceCode.getText(node)
})
return
}
}
}
}
})

function getFindByQueryVariant(queryMethod: string) {
return queryMethod.includes('All') ? 'findAllBy' : 'findBy'
}
80 changes: 80 additions & 0 deletions tests/lib/rules/prefer-find-by.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { InvalidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
import { createRuleTester } from '../test-utils';
import { ASYNC_QUERIES_COMBINATIONS, SYNC_QUERIES_COMBINATIONS } from '../../../lib/utils';
import rule, { WAIT_METHODS, RULE_NAME } from '../../../lib/rules/prefer-find-by';

const ruleTester = createRuleTester({
ecmaFeatures: {
jsx: true,
},
});

ruleTester.run(RULE_NAME, rule, {
valid: [
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `const submitButton = await ${queryMethod}('foo')`
})),
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `const submitButton = await screen.${queryMethod}('foo')`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `await waitForElementToBeRemoved(() => ${queryMethod}(baz))`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `await waitFor(function() {
return ${queryMethod}('baz', { name: 'foo' })
})`
})),
{
code: `await waitFor(() => myCustomFunction())`
},
{
code: `await waitFor(customFunctionReference)`
},
{
code: `await waitForElementToBeRemoved(document.querySelector('foo'))`
},
...SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `
await waitFor(() => {
foo()
return ${queryMethod}()
})
`
})),
],
invalid: [
// using reduce + concat 'cause flatMap is not available in node10.x
...WAIT_METHODS.reduce((acc: InvalidTestCase<'preferFindBy', []>[], waitMethod) => acc
.concat(
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `
const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
}
}]
}))
).concat(
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `
const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
}
}]
}))
),
[])
],
})

0 comments on commit 38a67f7

Please sign in to comment.