diff --git a/README.md b/README.md index a1aa0bdb..fff1c2ff 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ -[![All Contributors](https://img.shields.io/badge/all_contributors-4-orange.svg?style=flat-square)](#contributors) +[![All Contributors](https://img.shields.io/badge/all_contributors-4-orange.svg?style=flat-square)](#contributors-) @@ -137,6 +137,7 @@ To enable this configuration use the `extends` property in your | 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][] | | | [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][] | | diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md new file mode 100644 index 00000000..b83a1d38 --- /dev/null +++ b/docs/rules/await-async-utils.md @@ -0,0 +1,72 @@ +# Enforce async utils to be awaited properly (await-async-utils) + +Ensure that promises returned by async utils are handled properly. + +## Rule Details + +Testing library provides several utilities for dealing with asynchronous code. These are useful to wait for an element until certain criteria or situation happens. The available async utils are: + +- `wait` +- `waitForElement` +- `waitForDomChange` +- `waitForElementToBeRemoved` + +This rule aims to prevent users from forgetting to handle the returned promise from those async utils, which could lead to unexpected errors in the tests execution. The promises can be handled by using either `await` operator or `then` method. + +Examples of **incorrect** code for this rule: + +```js +test('something incorrectly', async () => { + // ... + wait(() => getByLabelText('email')); + + const [usernameElement, passwordElement] = waitForElement( + () => [ + getByLabelText(container, 'username'), + getByLabelText(container, 'password'), + ], + { container } + ); + + waitForDomChange(() => { + return { container }; + }); + + waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')); + // ... +}); +``` + +Examples of **correct** code for this rule: + +```js +test('something correctly', async () => { + // ... + // `await` operator is correct + await wait(() => getByLabelText('email')); + + const [usernameElement, passwordElement] = await waitForElement( + () => [ + getByLabelText(container, 'username'), + getByLabelText(container, 'password'), + ], + { container } + ); + + // `then` chained method is correct + waitForDomChange(() => { + return { container }; + }) + .then(() => console.log('DOM changed!')) + .catch(err => console.log(`Error you need to deal with: ${err}`)); + + // return the promise within a function is correct too! + const makeCustomWait = () => + waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')); + // ... +}); +``` + +## Further Reading + +- [Async Utilities](https://testing-library.com/docs/dom-testing-library/api-async) diff --git a/lib/index.js b/lib/index.js index 6c25b8b2..c8f17612 100644 --- a/lib/index.js +++ b/lib/index.js @@ -2,6 +2,7 @@ const rules = { 'await-async-query': require('./rules/await-async-query'), + 'await-async-utils': require('./rules/await-async-utils'), 'await-fire-event': require('./rules/await-fire-event'), 'consistent-data-testid': require('./rules/consistent-data-testid'), 'no-await-sync-query': require('./rules/no-await-sync-query'), @@ -13,6 +14,7 @@ const rules = { const recommendedRules = { 'testing-library/await-async-query': 'error', + 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', }; diff --git a/lib/rules/await-async-utils.js b/lib/rules/await-async-utils.js new file mode 100644 index 00000000..1ddc9936 --- /dev/null +++ b/lib/rules/await-async-utils.js @@ -0,0 +1,103 @@ +'use strict'; + +const { getDocsUrl, ASYNC_UTILS } = require('../utils'); + +const VALID_PARENTS = [ + 'AwaitExpression', + 'ArrowFunctionExpression', + 'ReturnStatement', +]; + +const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`); + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Enforce async utils to be awaited properly', + category: 'Best Practices', + recommended: true, + url: getDocsUrl('await-async-utils'), + }, + messages: { + awaitAsyncUtil: 'Promise returned from `{{ name }}` must be handled', + }, + fixable: null, + schema: [], + }, + + create: function(context) { + const testingLibraryUtilUsage = []; + return { + [`CallExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`](node) { + if (!isAwaited(node.parent.parent) && !isPromiseResolved(node)) { + testingLibraryUtilUsage.push(node); + } + }, + 'Program:exit'() { + testingLibraryUtilUsage.forEach(node => { + const variableDeclaratorParent = node.parent.parent; + + const references = + (variableDeclaratorParent.type === 'VariableDeclarator' && + context + .getDeclaredVariables(variableDeclaratorParent)[0] + .references.slice(1)) || + []; + + if ( + references && + references.length === 0 && + !isAwaited(node.parent.parent) && + !isPromiseResolved(node) + ) { + context.report({ + node, + messageId: 'awaitAsyncUtil', + data: { + name: node.name, + }, + }); + } else { + for (const reference of references) { + const referenceNode = reference.identifier; + if ( + !isAwaited(referenceNode.parent) && + !isPromiseResolved(referenceNode) + ) { + context.report({ + node, + messageId: 'awaitAsyncUtil', + data: { + name: node.name, + }, + }); + + break; + } + } + } + }); + }, + }; + }, +}; + +function isAwaited(node) { + return VALID_PARENTS.includes(node.type); +} + +const hasAThenProperty = node => + node.type === 'MemberExpression' && node.property.name === 'then'; + +function isPromiseResolved(node) { + const parent = node.parent; + + // wait(...).then(...) + if (parent.type === 'CallExpression') { + return hasAThenProperty(parent.parent); + } + + // promise.then(...) + return hasAThenProperty(parent); +} diff --git a/lib/utils.js b/lib/utils.js index 55ec08d7..8f477be4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -48,6 +48,13 @@ const ALL_QUERIES_COMBINATIONS = [ ASYNC_QUERIES_COMBINATIONS, ]; +const ASYNC_UTILS = [ + 'wait', + 'waitForElement', + 'waitForDomChange', + 'waitForElementToBeRemoved', +]; + module.exports = { getDocsUrl, SYNC_QUERIES_VARIANTS, @@ -57,4 +64,5 @@ module.exports = { SYNC_QUERIES_COMBINATIONS, ASYNC_QUERIES_COMBINATIONS, ALL_QUERIES_COMBINATIONS, + ASYNC_UTILS, }; diff --git a/tests/__snapshots__/index.test.js.snap b/tests/__snapshots__/index.test.js.snap index b83bb366..86bf0861 100644 --- a/tests/__snapshots__/index.test.js.snap +++ b/tests/__snapshots__/index.test.js.snap @@ -7,6 +7,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ @@ -24,6 +25,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ @@ -41,6 +43,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", }, } @@ -53,6 +56,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/await-fire-event": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", diff --git a/tests/lib/rules/await-async-utils.js b/tests/lib/rules/await-async-utils.js new file mode 100644 index 00000000..986ce047 --- /dev/null +++ b/tests/lib/rules/await-async-utils.js @@ -0,0 +1,141 @@ +'use strict'; + +const rule = require('../../../lib/rules/await-async-utils'); +const { ASYNC_UTILS } = require('../../../lib/utils'); +const RuleTester = require('eslint').RuleTester; + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } }); + +ruleTester.run('await-async-utils', rule, { + valid: [ + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly waited with await operator is valid', async () => { + doSomethingElse(); + await ${asyncUtil}(() => getByLabelText('email')); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and waited with await operator is valid', async () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + await aPromise; + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly chained with then is valid', () => { + doSomethingElse(); + ${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') }); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and chained with then is valid', () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + aPromise.then(() => { console.log('done') }); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly returned in arrow function is valid', async () => { + const makeCustomWait = () => + ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util explicitly returned in arrow function is valid', async () => { + const makeCustomWait = () => { + return ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + }; + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util returned in regular function is valid', async () => { + function makeCustomWait() { + return ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + } + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and returned in function is valid', async () => { + const makeCustomWait = () => { + const aPromise = ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + + doSomethingElse(); + + return aPromise; + }; + }); + `, + })), + { + code: ` + test('util not related to testing library is valid', async () => { + doSomethingElse(); + waitNotRelatedToTestingLibrary(); + }); + `, + }, + ], + invalid: [ + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util not waited', () => { + doSomethingElse(); + ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [{ line: 4, messageId: 'awaitAsyncUtil' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved not waited', () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [{ line: 4, column: 28, messageId: 'awaitAsyncUtil' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('several ${asyncUtil} utils not waited', () => { + const aPromise = ${asyncUtil}(() => getByLabelText('username')); + doSomethingElse(aPromise); + ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [ + { line: 3, column: 28, messageId: 'awaitAsyncUtil' }, + { line: 5, column: 11, messageId: 'awaitAsyncUtil' }, + ], + })), + ], +});