diff --git a/README.md b/README.md index 7995b942..e863cd1f 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,7 @@ To enable this configuration use the `extends` property in your | [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | | | [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | +| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [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 | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | diff --git a/docs/rules/no-side-effects-wait-for.md b/docs/rules/no-side-effects-wait-for.md new file mode 100644 index 00000000..05cecb33 --- /dev/null +++ b/docs/rules/no-side-effects-wait-for.md @@ -0,0 +1,71 @@ +# Side effects inside `waitFor` are not preferred (no-side-effects-wait-for) + +## Rule Details + +This rule aims to avoid the usage of side effects actions (`fireEvent` or `userEvent`) inside `waitFor`. +Since `waitFor` is intended for things that have a non-deterministic amount of time between the action you performed and the assertion passing, +the callback can be called (or checked for errors) a non-deterministic number of times and frequency. +This will make your side-effect run multiple times. + +Example of **incorrect** code for this rule: + +```js + await waitFor(() => { + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(b).toEqual('b'); + }); + + // or + await waitFor(function() { + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(b).toEqual('b'); + }); + + // or + await waitFor(() => { + userEvent.click(button); + expect(b).toEqual('b'); + }); + + // or + await waitFor(function() { + userEvent.click(button); + expect(b).toEqual('b'); + }); +}; +``` + +Examples of **correct** code for this rule: + +```js + fireEvent.keyDown(input, { key: 'ArrowDown' }); + await waitFor(() => { + expect(b).toEqual('b'); + }); + + // or + fireEvent.keyDown(input, { key: 'ArrowDown' }); + await waitFor(function() { + expect(b).toEqual('b'); + }); + + // or + userEvent.click(button); + await waitFor(() => { + expect(b).toEqual('b'); + }); + + // or + userEvent.click(button); + await waitFor(function() { + expect(b).toEqual('b'); + }); +}; +``` + +## Further Reading + +- [about `waitFor`](https://testing-library.com/docs/dom-testing-library/api-async#waitfor) +- [about `userEvent`](https://github.com/testing-library/user-event) +- [about `fireEvent`](https://testing-library.com/docs/dom-testing-library/api-events) +- [inspiration for this rule](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#performing-side-effects-in-waitfor) diff --git a/lib/index.ts b/lib/index.ts index d8a64006..29559c1b 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -17,6 +17,7 @@ import preferUserEvent from './rules/prefer-user-event'; import preferWaitFor from './rules/prefer-wait-for'; import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for'; import preferFindBy from './rules/prefer-find-by'; +import noSideEffectsWaitFor from './rules/no-side-effects-wait-for'; import renderResultNamingConvention from './rules/render-result-naming-convention'; const rules = { @@ -32,6 +33,7 @@ const rules = { 'no-multiple-assertions-wait-for': noMultipleAssertionsWaitFor, 'no-node-access': noNodeAccess, 'no-promise-in-fire-event': noPromiseInFireEvent, + 'no-side-effects-wait-for': noSideEffectsWaitFor, 'no-wait-for-empty-callback': noWaitForEmptyCallback, 'prefer-explicit-assert': preferExplicitAssert, 'prefer-find-by': preferFindBy, diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts new file mode 100644 index 00000000..7fbeed36 --- /dev/null +++ b/lib/rules/no-side-effects-wait-for.ts @@ -0,0 +1,70 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils' +import { getDocsUrl, hasTestingLibraryImportModule } from '../utils' +import { isBlockStatement, findClosestCallNode, isMemberExpression, isCallExpression, isIdentifier } from '../node-utils' + +export const RULE_NAME = 'no-side-effects-wait-for'; + +const WAIT_EXPRESSION_QUERY = + 'CallExpression[callee.name=/^(waitFor)$/]'; + +const SIDE_EFFECTS: Array = ['fireEvent', 'userEvent'] + +export type MessageIds = 'noSideEffectsWaitFor'; +type Options = []; + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: + "It's preferred to avoid side effects in `waitFor`", + category: 'Best Practices', + recommended: false, + }, + messages: { + noSideEffectsWaitFor: 'Avoid using side effects within `waitFor` callback', + }, + fixable: null, + schema: [], + }, + defaultOptions: [], + create: function(context) { + let isImportingTestingLibrary = false; + + function reportSideEffects( + node: TSESTree.BlockStatement + ) { + const hasSideEffects = (body: Array): boolean => + body.some((node: TSESTree.ExpressionStatement) => { + if ( + isCallExpression(node.expression) && + isMemberExpression(node.expression.callee) && + isIdentifier(node.expression.callee.object) + ) { + const object: TSESTree.Identifier = node.expression.callee.object + const identifierName: string = object.name + return SIDE_EFFECTS.includes(identifierName) + } else { + return false + } + }) + + if (isImportingTestingLibrary && isBlockStatement(node) && hasSideEffects(node.body)) { + context.report({ + node, + loc: node.loc.start, + messageId: 'noSideEffectsWaitFor', + }); + } + } + + return { + [`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression > BlockStatement`]: reportSideEffects, + [`${WAIT_EXPRESSION_QUERY} > FunctionExpression > BlockStatement`]: reportSideEffects, + ImportDeclaration(node: TSESTree.ImportDeclaration) { + isImportingTestingLibrary = hasTestingLibraryImportModule(node); + } + }; + } +}) diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts new file mode 100644 index 00000000..560db55d --- /dev/null +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -0,0 +1,226 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-side-effects-wait-for'; + +const ruleTester = createRuleTester({ + ecmaFeatures: { + jsx: true, + }, +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => expect(a).toEqual('a')) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + expect(a).toEqual('a') + }) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + console.log('testing-library') + expect(b).toEqual('b') + }) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + console.log('testing-library') + expect(b).toEqual('b') + }) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => {}) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() {}) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + // testing + }) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + // testing + }) + `, + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + fireEvent.keyDown(input, {key: 'ArrowDown'}) + await waitFor(() => { + expect(b).toEqual('b') + }) + ` + }, { + code: ` + import { waitFor } from '@testing-library/react'; + fireEvent.keyDown(input, {key: 'ArrowDown'}) + await waitFor(function() { + expect(b).toEqual('b') + }) + ` + }, { + code: ` + import { waitFor } from '@testing-library/react'; + userEvent.click(button) + await waitFor(function() { + expect(b).toEqual('b') + }) + ` + }, { + code: ` + import { waitFor } from 'react'; + await waitFor(function() { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + expect(b).toEqual('b') + }) + ` + } + ], + invalid: [ + // fireEvent + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + expect(b).toEqual('b') + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + expect(b).toEqual('b') + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + expect(b).toEqual('b') + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + expect(b).toEqual('b') + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + // userEvent + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + userEvent.click(button) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + expect(b).toEqual('b') + userEvent.click(button) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(() => { + userEvent.click(button) + expect(b).toEqual('b') + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + userEvent.click(button) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + expect(b).toEqual('b') + userEvent.click(button) + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + }, + { + code: ` + import { waitFor } from '@testing-library/react'; + await waitFor(function() { + userEvent.click(button) + expect(b).toEqual('b') + }) + `, + errors: [{ messageId: 'noSideEffectsWaitFor' }] + } + ] +})