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 31, 2020
1 parent d4a924d commit e649e9d
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 15 deletions.
34 changes: 19 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
[![Tweet][tweet-badge]][tweet-url]

<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->

[![All Contributors](https://img.shields.io/badge/all_contributors-20-orange.svg?style=flat-square)](#contributors-)

<!-- ALL-CONTRIBUTORS-BADGE:END -->

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

## Supported Rules

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

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

<!-- markdownlint-enable -->
<!-- prettier-ignore-end -->

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
78 changes: 78 additions & 0 deletions docs/rules/prefer-find-by.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# 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.getAllByTestId('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());

// using expects inside waitFor
await waitFor(() => expect(screen.getByText('bar').toBeDisabled());
await waitFor(() => expect(getAllByText('bar').toBeDisabled());
```
## 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';

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'
}
90 changes: 90 additions & 0 deletions tests/lib/rules/prefer-find-by.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
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) => ({
code: `const submitButton = await ${queryMethod}('foo')`
})),
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `const submitButton = await screen.${queryMethod}('foo')`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `await waitForElementToBeRemoved(() => ${queryMethod}(baz))`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
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) => ({
code: `
await waitFor(() => {
foo()
return ${queryMethod}()
})
`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `
await waitFor(() => expect(screen.${queryMethod}('baz')).toBeDisabled());
`
})),
...SYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `
await waitFor(() => expect(${queryMethod}('baz')).toBeInTheDocument());
`
}))
],
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 e649e9d

Please sign in to comment.