feat: add no-side-effects-wait-for rule (#196)

* test: add scenarios for no-side-effects-wait-for * feat: add no-side-effects-wait-for rule * feat: add no-side-effects-wait-for in index * test: add more valid scenarios in no-side-effects-wait-for * docs: include no-side-effects-wait-for * fix: typo in no-side-effects-wait-for doc Co-authored-by: Gonzalo D'Elia <gonzalo.n.delia@gmail.com> * fix: remove extra code in examples * refactor: use some instead filter in no-side-effects-wait-for * feat: check if no-side-effects-wait-for is called inside tests * refactor: use util for import check at no-side-effects-wait-for * test: valid scenario for no TL wait for import at no-side-effects * refactor: usage of correct user event methods Co-authored-by: Gonzalo D'Elia <gonzalo.n.delia@gmail.com>
testing-library · Jul 28, 2020 · 91200b7 · 91200b7
1 parent 8ad0184
commit 91200b7
Show file tree

Hide file tree

Showing 5 changed files with 370 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<string> = ['fireEvent', 'userEvent']
+
+export type MessageIds = 'noSideEffectsWaitFor';
+type Options = [];
+
+export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
+  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<TSESTree.Node>): 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
@@ -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' }]
+    }
+  ]
+})