diff --git a/README.md b/README.md index 1eca05f6..7ba942dd 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-20-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -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][] ![angular-badge][] ![react-badge][] ![vue-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 @@ -205,6 +208,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d + This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/docs/rules/prefer-find-by.md b/docs/rules/prefer-find-by.md new file mode 100644 index 00000000..e24d585b --- /dev/null +++ b/docs/rules/prefer-find-by.md @@ -0,0 +1,78 @@ +# Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries (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) diff --git a/lib/index.ts b/lib/index.ts index 07f8a89b..3cc5ec21 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -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, @@ -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, @@ -32,6 +34,7 @@ const recommendedRules = { 'testing-library/await-async-query': 'error', 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', + 'testing-library/prefer-find-by': 'error', }; export = { diff --git a/lib/node-utils.ts b/lib/node-utils.ts index bf84eed7..b8d3efc4 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -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' +} \ No newline at end of file diff --git a/lib/rules/prefer-find-by.ts b/lib/rules/prefer-find-by.ts new file mode 100644 index 00000000..da1e627f --- /dev/null +++ b/lib/rules/prefer-find-by.ts @@ -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)({ + 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' +} \ No newline at end of file diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index 86bf0861..fedc3c05 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -14,6 +14,7 @@ Object { "error", "angular", ], + "testing-library/prefer-find-by": "error", }, } `; @@ -32,6 +33,7 @@ Object { "error", "react", ], + "testing-library/prefer-find-by": "error", }, } `; @@ -45,6 +47,7 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", + "testing-library/prefer-find-by": "error", }, } `; @@ -64,6 +67,7 @@ Object { "error", "vue", ], + "testing-library/prefer-find-by": "error", }, } `; diff --git a/tests/lib/rules/prefer-find-by.test.ts b/tests/lib/rules/prefer-find-by.test.ts new file mode 100644 index 00000000..dd0cf477 --- /dev/null +++ b/tests/lib/rules/prefer-find-by.test.ts @@ -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' }))`, + } + }] + })) + ), + []) + ], +}) \ No newline at end of file