diff --git a/README.md b/README.md index 203570bd..e72bb9b1 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-36-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -139,9 +141,8 @@ To enable this configuration use the `extends` property in your | [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-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | -| [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][] | | -| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | +| [no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | | [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-wait-for-side-effects.md similarity index 95% rename from docs/rules/no-side-effects-wait-for.md rename to docs/rules/no-wait-for-side-effects.md index 05cecb33..ccb3dd95 100644 --- a/docs/rules/no-side-effects-wait-for.md +++ b/docs/rules/no-wait-for-side-effects.md @@ -1,4 +1,4 @@ -# Side effects inside `waitFor` are not preferred (no-side-effects-wait-for) +# Side effects inside `waitFor` are not preferred (no-wait-for-side-effects) ## Rule Details diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 204b4f64..cc5686fa 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -96,6 +96,8 @@ export interface DetectionHelpers { isQuery: IsQueryFn; isCustomQuery: IsCustomQueryFn; isAsyncUtil: IsAsyncUtilFn; + isFireEventUtil: (node: TSESTree.Identifier) => boolean; + isUserEventUtil: (node: TSESTree.Identifier) => boolean; isFireEventMethod: IsFireEventMethodFn; isRenderUtil: IsRenderUtilFn; isRenderVariableDeclarator: IsRenderVariableDeclaratorFn; @@ -107,7 +109,6 @@ export interface DetectionHelpers { isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn; } -const FIRE_EVENT_NAME = 'fireEvent'; const RENDER_NAME = 'render'; /** @@ -173,6 +174,69 @@ export function detectTestingLibraryUtils< return isNodeComingFromTestingLibrary(referenceNodeIdentifier); } + /** + * Determines whether a given node is a simulate event util related to + * Testing Library or not. + * + * In order to determine this, the node must match: + * - indicated simulate event name: fireEvent or userEvent + * - imported from valid Testing Library module (depends on Aggressive + * Reporting) + * + */ + function isTestingLibrarySimulateEventUtil( + node: TSESTree.Identifier, + utilName: 'fireEvent' | 'userEvent' + ): boolean { + const simulateEventUtil = findImportedUtilSpecifier(utilName); + let simulateEventUtilName: string | undefined; + + if (simulateEventUtil) { + simulateEventUtilName = ASTUtils.isIdentifier(simulateEventUtil) + ? simulateEventUtil.name + : simulateEventUtil.local.name; + } else if (isAggressiveModuleReportingEnabled()) { + simulateEventUtilName = utilName; + } + + if (!simulateEventUtilName) { + return false; + } + + const parentMemberExpression: + | TSESTree.MemberExpression + | undefined = isMemberExpression(node.parent) ? node.parent : undefined; + + if (!parentMemberExpression) { + return false; + } + + // make sure that given node it's not fireEvent/userEvent object itself + if ( + [simulateEventUtilName, utilName].includes(node.name) || + (ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === node.name) + ) { + return false; + } + + // check fireEvent.click()/userEvent.click() usage + const regularCall = + ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === simulateEventUtilName; + + // check testingLibraryUtils.fireEvent.click() or + // testingLibraryUtils.userEvent.click() usage + const wildcardCall = + isMemberExpression(parentMemberExpression.object) && + ASTUtils.isIdentifier(parentMemberExpression.object.object) && + parentMemberExpression.object.object.name === simulateEventUtilName && + ASTUtils.isIdentifier(parentMemberExpression.object.property) && + parentMemberExpression.object.property.name === utilName; + + return regularCall || wildcardCall; + } + /** * Determines whether aggressive module reporting is enabled or not. * @@ -308,55 +372,38 @@ export function detectTestingLibraryUtils< }; /** - * Determines whether a given node is fireEvent method or not + * Determines whether a given node is fireEvent util itself or not. + * + * Not to be confused with {@link isFireEventMethod} */ - const isFireEventMethod: IsFireEventMethodFn = (node) => { - const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME); - let fireEventUtilName: string | undefined; - - if (fireEventUtil) { - fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil) - ? fireEventUtil.name - : fireEventUtil.local.name; - } else if (isAggressiveModuleReportingEnabled()) { - fireEventUtilName = FIRE_EVENT_NAME; - } - - if (!fireEventUtilName) { - return false; - } - - const parentMemberExpression: - | TSESTree.MemberExpression - | undefined = isMemberExpression(node.parent) ? node.parent : undefined; - - if (!parentMemberExpression) { - return false; - } - - // make sure that given node it's not fireEvent object itself - if ( - [fireEventUtilName, FIRE_EVENT_NAME].includes(node.name) || - (ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === node.name) - ) { - return false; - } - - // check fireEvent.click() usage - const regularCall = - ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === fireEventUtilName; + const isFireEventUtil = (node: TSESTree.Identifier): boolean => { + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName].includes('fireEvent'); + } + ); + }; - // check testingLibraryUtils.fireEvent.click() usage - const wildcardCall = - isMemberExpression(parentMemberExpression.object) && - ASTUtils.isIdentifier(parentMemberExpression.object.object) && - parentMemberExpression.object.object.name === fireEventUtilName && - ASTUtils.isIdentifier(parentMemberExpression.object.property) && - parentMemberExpression.object.property.name === FIRE_EVENT_NAME; + /** + * Determines whether a given node is userEvent util itself or not. + * + * Not to be confused with {@link isUserEventMethod} + */ + const isUserEventUtil = (node: TSESTree.Identifier): boolean => { + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName].includes('userEvent'); + } + ); + }; - return regularCall || wildcardCall; + /** + * Determines whether a given node is fireEvent method or not + */ + const isFireEventMethod: IsFireEventMethodFn = (node) => { + return isTestingLibrarySimulateEventUtil(node, 'fireEvent'); }; /** @@ -557,6 +604,8 @@ export function detectTestingLibraryUtils< isQuery, isCustomQuery, isAsyncUtil, + isFireEventUtil, + isUserEventUtil, isFireEventMethod, isRenderUtil, isRenderVariableDeclarator, diff --git a/lib/index.ts b/lib/index.ts index f722c9eb..6ca532a2 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -20,7 +20,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 noWaitForSideEffects from './rules/no-wait-for-side-effects'; import renderResultNamingConvention from './rules/render-result-naming-convention'; const rules = { @@ -38,8 +38,8 @@ const rules = { 'no-node-access': noNodeAccess, 'no-promise-in-fire-event': noPromiseInFireEvent, 'no-render-in-setup': noRenderInSetup, - 'no-side-effects-wait-for': noSideEffectsWaitFor, 'no-wait-for-empty-callback': noWaitForEmptyCallback, + 'no-wait-for-side-effects': noWaitForSideEffects, 'no-wait-for-snapshot': noWaitForSnapshot, 'prefer-explicit-assert': preferExplicitAssert, 'prefer-find-by': preferFindBy, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index dc0dde00..07a9d3ff 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -101,6 +101,12 @@ export function isJSXAttribute( return node?.type === AST_NODE_TYPES.JSXAttribute; } +export function isExpressionStatement( + node: TSESTree.Node +): node is TSESTree.ExpressionStatement { + return node?.type === AST_NODE_TYPES.ExpressionStatement; +} + /** * Finds the closest CallExpression node for a given node. * @param node @@ -388,6 +394,10 @@ export function getPropertyIdentifierNode( return getPropertyIdentifierNode(node.callee); } + if (isExpressionStatement(node)) { + return getPropertyIdentifierNode(node.expression); + } + return null; } diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts deleted file mode 100644 index d3b55504..00000000 --- a/lib/rules/no-side-effects-wait-for.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, hasTestingLibraryImportModule } from '../utils'; -import { - isBlockStatement, - isMemberExpression, - isCallExpression, -} from '../node-utils'; - -export const RULE_NAME = 'no-side-effects-wait-for'; -export type MessageIds = 'noSideEffectsWaitFor'; -type Options = []; - -const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; - -const SIDE_EFFECTS: Array = ['fireEvent', 'userEvent']; - -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) && - ASTUtils.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/lib/rules/no-wait-for-side-effects.ts b/lib/rules/no-wait-for-side-effects.ts new file mode 100644 index 00000000..a502d2b7 --- /dev/null +++ b/lib/rules/no-wait-for-side-effects.ts @@ -0,0 +1,67 @@ +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { getPropertyIdentifierNode } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; + +export const RULE_NAME = 'no-wait-for-side-effects'; +export type MessageIds = 'noSideEffectsWaitFor'; +type Options = []; + +export default createTestingLibraryRule({ + 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, _, helpers) { + function hasSideEffects(body: Array): boolean { + return body.some((node: TSESTree.ExpressionStatement) => { + const expressionIdentifier = getPropertyIdentifierNode(node); + + if (!expressionIdentifier) { + return false; + } + + return ( + helpers.isFireEventUtil(expressionIdentifier) || + helpers.isUserEventUtil(expressionIdentifier) + ); + }); + } + + function reportSideEffects(node: TSESTree.BlockStatement) { + const callExpressionNode = node.parent.parent as TSESTree.CallExpression; + const callExpressionIdentifier = getPropertyIdentifierNode( + callExpressionNode + ); + + if (!helpers.isAsyncUtil(callExpressionIdentifier, ['waitFor'])) { + return; + } + + if (!hasSideEffects(node.body)) { + return; + } + + context.report({ + node: callExpressionNode, + messageId: 'noSideEffectsWaitFor', + }); + } + + return { + 'CallExpression > ArrowFunctionExpression > BlockStatement': reportSideEffects, + 'CallExpression > FunctionExpression > BlockStatement': reportSideEffects, + }; + }, +}); diff --git a/lib/utils.ts b/lib/utils.ts index a464a517..1e4f0064 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,5 +1,3 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; - const combineQueries = (variants: string[], methods: string[]): string[] => { const combinedQueries: string[] = []; variants.forEach((variant) => { @@ -24,13 +22,6 @@ const LIBRARY_MODULES = [ '@testing-library/svelte', ]; -// TODO: should be deleted after all rules are migrated to v4 -const hasTestingLibraryImportModule = ( - node: TSESTree.ImportDeclaration -): boolean => { - return LIBRARY_MODULES.includes(node.source.value.toString()); -}; - const SYNC_QUERIES_VARIANTS = ['getBy', 'getAllBy', 'queryBy', 'queryAllBy']; const ASYNC_QUERIES_VARIANTS = ['findBy', 'findAllBy']; const ALL_QUERIES_VARIANTS = [ @@ -117,7 +108,6 @@ const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; export { combineQueries, getDocsUrl, - hasTestingLibraryImportModule, SYNC_QUERIES_VARIANTS, ASYNC_QUERIES_VARIANTS, ALL_QUERIES_VARIANTS, diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-wait-for-side-effects.test.ts similarity index 54% rename from tests/lib/rules/no-side-effects-wait-for.test.ts rename to tests/lib/rules/no-wait-for-side-effects.test.ts index 6b3fadf2..a72a30b9 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-wait-for-side-effects.test.ts @@ -1,5 +1,5 @@ import { createRuleTester } from '../test-utils'; -import rule, { RULE_NAME } from '../../../lib/rules/no-side-effects-wait-for'; +import rule, { RULE_NAME } from '../../../lib/rules/no-wait-for-side-effects'; const ruleTester = createRuleTester(); @@ -93,14 +93,71 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` - import { waitFor } from 'react'; + import { waitFor } from 'somewhere-else'; await waitFor(function() { fireEvent.keyDown(input, {key: 'ArrowDown'}) expect(b).toEqual('b') }) `, }, + { + code: ` + import { waitFor } from '@testing-library/react'; + + anotherFunction(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}); + userEvent.click(button); + }); + + test('side effects in functions other than waitFor are valid', () => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + expect(b).toEqual('b') + }); + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor } from 'somewhere-else'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor as renamedWaitFor, fireEvent, userEvent } from 'test-utils'; + import { waitFor } from 'somewhere-else'; + + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + }) + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor, fireEvent as renamedFireEvent, userEvent as renamedUserEvent } from 'test-utils'; + import { fireEvent, userEvent } from 'somewhere-else'; + + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + }) + `, + }, + { + code: `// weird case to cover 100% coverage + await waitFor(() => { + const click = firEvent['click'] + }) + `, + }, ], invalid: [ // fireEvent @@ -111,7 +168,26 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, + { + code: ` + import { waitFor, fireEvent as renamedFireEvent } from '@testing-library/react'; + await waitFor(() => { + renamedFireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, + { + settings: { 'testing-library/utils-module': '~/test-utils' }, + code: ` + import { waitFor, fireEvent } from '~/test-utils'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -121,7 +197,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -131,7 +207,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -140,7 +216,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -150,7 +226,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -160,7 +236,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, // userEvent { @@ -170,7 +246,26 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, + { + code: ` + import { waitFor, userEvent as renamedUserEvent } from '@testing-library/react'; + await waitFor(() => { + renamedUserEvent.click(button) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, + { + settings: { 'testing-library/utils-module': '~/test-utils' }, + code: ` + import { waitFor, userEvent } from '~/test-utils'; + await waitFor(() => { + userEvent.click(); + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -180,7 +275,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -190,7 +285,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -199,7 +294,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -209,7 +304,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -219,7 +314,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, ], });