diff --git a/README.md b/README.md index 43895ff4..612210d9 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ To enable this configuration use the `extends` property in your | [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-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![recommended-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 | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | diff --git a/docs/rules/no-promise-in-fire-event.md b/docs/rules/no-promise-in-fire-event.md new file mode 100644 index 00000000..3501e3a6 --- /dev/null +++ b/docs/rules/no-promise-in-fire-event.md @@ -0,0 +1,35 @@ +# Disallow the use of promises passed to a `fireEvent` method (no-promise-in-fire-event) + +The `fireEvent` method expects that a DOM element is passed. + +Examples of **incorrect** code for this rule: + +```js +import { screen, fireEvent } from '@testing-library/react'; + +// usage of findBy queries +fireEvent.click(screen.findByRole('button')); + +// usage of promises +fireEvent.click(new Promise(jest.fn()) +``` + +Examples of **correct** code for this rule: + +```js +import { screen, fireEvent } from '@testing-library/react'; + +// use getBy queries +fireEvent.click(screen.getByRole('button')); + +// use awaited findBy queries +fireEvent.click(await screen.findByRole('button')); + +// this won't give a linting error, but it will throw a runtime error +const promise = new Promise(); +fireEvent.click(promise)`, +``` + +## Further Reading + +- [A Github Issue explaining the problem](https://github.com/testing-library/dom-testing-library/issues/609) diff --git a/lib/index.ts b/lib/index.ts index 22996a13..886c2cd6 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -7,6 +7,7 @@ import noDebug from './rules/no-debug'; import noDomImport from './rules/no-dom-import'; import noManualCleanup from './rules/no-manual-cleanup'; import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback'; +import noPromiseInFireEvent from './rules/no-promise-in-fire-event'; import preferExplicitAssert from './rules/prefer-explicit-assert'; import preferPresenceQueries from './rules/prefer-presence-queries'; import preferScreenQueries from './rules/prefer-screen-queries'; @@ -22,6 +23,7 @@ const rules = { 'no-debug': noDebug, 'no-dom-import': noDomImport, 'no-manual-cleanup': noManualCleanup, + 'no-promise-in-fire-event': noPromiseInFireEvent, 'no-wait-for-empty-callback': noWaitForEmptyCallback, 'prefer-explicit-assert': preferExplicitAssert, 'prefer-find-by': preferFindBy, @@ -34,6 +36,7 @@ const recommendedRules = { 'testing-library/await-async-query': 'error', 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', + 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-wait-for-empty-callback': 'error', 'testing-library/prefer-find-by': 'error', 'testing-library/prefer-screen-queries': 'error', diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 53e788b8..544df8fc 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -12,6 +12,12 @@ export function isAwaitExpression( return node && node.type === 'AwaitExpression'; } +export function isNewExpression( + node: TSESTree.Node +): node is TSESTree.NewExpression { + return node && node.type === 'NewExpression'; +} + export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { return node && node.type === 'Identifier'; } @@ -103,6 +109,8 @@ export function hasThenProperty(node: TSESTree.Node) { ); } -export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression { - return node && node.type === 'ArrowFunctionExpression' -} \ No newline at end of file +export function isArrowFunctionExpression( + node: TSESTree.Node +): node is TSESTree.ArrowFunctionExpression { + return node && node.type === 'ArrowFunctionExpression'; +} diff --git a/lib/rules/no-promise-in-fire-event.ts b/lib/rules/no-promise-in-fire-event.ts new file mode 100644 index 00000000..3221a3f1 --- /dev/null +++ b/lib/rules/no-promise-in-fire-event.ts @@ -0,0 +1,74 @@ +import { TSESTree, ESLintUtils } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils'; +import { + isNewExpression, + isIdentifier, + isImportSpecifier, + isCallExpression, +} from '../node-utils'; + +export const RULE_NAME = 'no-promise-in-fire-event'; +export type MessageIds = 'noPromiseInFireEvent'; +type Options = []; + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow the use of promises passed to a `fireEvent` method', + category: 'Best Practices', + recommended: false, + }, + messages: { + noPromiseInFireEvent: + "A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element", + }, + fixable: 'code', + schema: [], + }, + defaultOptions: [], + + create(context) { + return { + 'ImportDeclaration[source.value=/testing-library/]'( + node: TSESTree.ImportDeclaration + ) { + const fireEventImportNode = node.specifiers.find( + specifier => + isImportSpecifier(specifier) && + specifier.imported && + 'fireEvent' === specifier.imported.name + ) as TSESTree.ImportSpecifier; + + const { references } = context.getDeclaredVariables( + fireEventImportNode + )[0]; + + for (const reference of references) { + const referenceNode = reference.identifier; + const callExpression = referenceNode.parent + .parent as TSESTree.CallExpression; + const [element] = callExpression.arguments as TSESTree.Node[]; + if (isCallExpression(element) || isNewExpression(element)) { + const methodName = isIdentifier(element.callee) + ? element.callee.name + : ((element.callee as TSESTree.MemberExpression) + .property as TSESTree.Identifier).name; + + if ( + ASYNC_QUERIES_VARIANTS.some(q => methodName.startsWith(q)) || + methodName === 'Promise' + ) { + context.report({ + node: element, + messageId: 'noPromiseInFireEvent', + }); + } + } + } + }, + }; + }, +}); diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index 17d3f1f9..0113c3ea 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -14,6 +14,7 @@ Object { "error", "angular", ], + "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/prefer-find-by": "error", "testing-library/prefer-screen-queries": "error", @@ -35,6 +36,7 @@ Object { "error", "react", ], + "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/prefer-find-by": "error", "testing-library/prefer-screen-queries": "error", @@ -51,6 +53,7 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", + "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/prefer-find-by": "error", "testing-library/prefer-screen-queries": "error", @@ -73,6 +76,7 @@ Object { "error", "vue", ], + "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/prefer-find-by": "error", "testing-library/prefer-screen-queries": "error", diff --git a/tests/lib/rules/no-promise-in-fire-event.test.ts b/tests/lib/rules/no-promise-in-fire-event.test.ts new file mode 100644 index 00000000..0ec11de3 --- /dev/null +++ b/tests/lib/rules/no-promise-in-fire-event.test.ts @@ -0,0 +1,94 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-promise-in-fire-event'; + +const ruleTester = createRuleTester(); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(screen.getByRole('button')) + `, + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(queryByRole('button'))`, + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(someRef)`, + }, + { + code: `fireEvent.click(findByText('submit'))`, + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + const promise = new Promise(); + fireEvent.click(promise)`, + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(await screen.findByRole('button')) + `, + }, + { + code: `fireEvent.click(Promise())`, + }, + ], + invalid: [ + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(screen.findByRole('button'))`, + errors: [ + { + messageId: 'noPromiseInFireEvent', + }, + ], + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(findByText('submit'))`, + errors: [ + { + messageId: 'noPromiseInFireEvent', + }, + ], + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(Promise('foo'))`, + errors: [ + { + messageId: 'noPromiseInFireEvent', + }, + ], + }, + { + code: ` + import {fireEvent} from '@testing-library/foo'; + + fireEvent.click(new Promise('foo'))`, + errors: [ + { + messageId: 'noPromiseInFireEvent', + }, + ], + }, + ], +});