Skip to content

Commit a4cc8d8

Browse files
timdeschryvertimdeschryver
authored
feat(no-promise-in-fire-event): add new no-promise-in-fire-event rule (#180)
* feat(no-promise-in-fire-event): add new no-promise-in-fire-event rule * test: 100% code coverage * docs: add rule to readme * chore: review changes * chore: add rule to recommended config
1 parent 63f2db4 commit a4cc8d8

File tree

7 files changed

+222
-3
lines changed

7 files changed

+222
-3
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ To enable this configuration use the `extends` property in your
143143
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
144144
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
145145
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
146+
| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | |
146147
| [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][] | |
147148
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
148149
| [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][] |

‎docs/rules/no-promise-in-fire-event.md

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Disallow the use of promises passed to a `fireEvent` method (no-promise-in-fire-event)
2+
3+
The `fireEvent` method expects that a DOM element is passed.
4+
5+
Examples of **incorrect** code for this rule:
6+
7+
```js
8+
import { screen, fireEvent } from '@testing-library/react';
9+
10+
// usage of findBy queries
11+
fireEvent.click(screen.findByRole('button'));
12+
13+
// usage of promises
14+
fireEvent.click(new Promise(jest.fn())
15+
```
16+
17+
Examples of **correct** code for this rule:
18+
19+
```js
20+
import { screen, fireEvent } from '@testing-library/react';
21+
22+
// use getBy queries
23+
fireEvent.click(screen.getByRole('button'));
24+
25+
// use awaited findBy queries
26+
fireEvent.click(await screen.findByRole('button'));
27+
28+
// this won't give a linting error, but it will throw a runtime error
29+
const promise = new Promise();
30+
fireEvent.click(promise)`,
31+
```
32+
33+
## Further Reading
34+
35+
- [A Github Issue explaining the problem](https://github.com/testing-library/dom-testing-library/issues/609)

‎lib/index.ts

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import noDebug from './rules/no-debug';
77
import noDomImport from './rules/no-dom-import';
88
import noManualCleanup from './rules/no-manual-cleanup';
99
import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback';
10+
import noPromiseInFireEvent from './rules/no-promise-in-fire-event';
1011
import preferExplicitAssert from './rules/prefer-explicit-assert';
1112
import preferPresenceQueries from './rules/prefer-presence-queries';
1213
import preferScreenQueries from './rules/prefer-screen-queries';
@@ -22,6 +23,7 @@ const rules = {
2223
'no-debug': noDebug,
2324
'no-dom-import': noDomImport,
2425
'no-manual-cleanup': noManualCleanup,
26+
'no-promise-in-fire-event': noPromiseInFireEvent,
2527
'no-wait-for-empty-callback': noWaitForEmptyCallback,
2628
'prefer-explicit-assert': preferExplicitAssert,
2729
'prefer-find-by': preferFindBy,
@@ -34,6 +36,7 @@ const recommendedRules = {
3436
'testing-library/await-async-query': 'error',
3537
'testing-library/await-async-utils': 'error',
3638
'testing-library/no-await-sync-query': 'error',
39+
'testing-library/no-promise-in-fire-event': 'error',
3740
'testing-library/no-wait-for-empty-callback': 'error',
3841
'testing-library/prefer-find-by': 'error',
3942
'testing-library/prefer-screen-queries': 'error',

‎lib/node-utils.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ export function isAwaitExpression(
1212
return node && node.type === 'AwaitExpression';
1313
}
1414

15+
export function isNewExpression(
16+
node: TSESTree.Node
17+
): node is TSESTree.NewExpression {
18+
return node && node.type === 'NewExpression';
19+
}
20+
1521
export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier {
1622
return node && node.type === 'Identifier';
1723
}
@@ -103,6 +109,8 @@ export function hasThenProperty(node: TSESTree.Node) {
103109
);
104110
}
105111

106-
export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
107-
return node && node.type === 'ArrowFunctionExpression'
108-
}
112+
export function isArrowFunctionExpression(
113+
node: TSESTree.Node
114+
): node is TSESTree.ArrowFunctionExpression {
115+
return node && node.type === 'ArrowFunctionExpression';
116+
}

‎lib/rules/no-promise-in-fire-event.ts

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { TSESTree, ESLintUtils } from '@typescript-eslint/experimental-utils';
2+
import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils';
3+
import {
4+
isNewExpression,
5+
isIdentifier,
6+
isImportSpecifier,
7+
isCallExpression,
8+
} from '../node-utils';
9+
10+
export const RULE_NAME = 'no-promise-in-fire-event';
11+
export type MessageIds = 'noPromiseInFireEvent';
12+
type Options = [];
13+
14+
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
15+
name: RULE_NAME,
16+
meta: {
17+
type: 'problem',
18+
docs: {
19+
description:
20+
'Disallow the use of promises passed to a `fireEvent` method',
21+
category: 'Best Practices',
22+
recommended: false,
23+
},
24+
messages: {
25+
noPromiseInFireEvent:
26+
"A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element",
27+
},
28+
fixable: 'code',
29+
schema: [],
30+
},
31+
defaultOptions: [],
32+
33+
create(context) {
34+
return {
35+
'ImportDeclaration[source.value=/testing-library/]'(
36+
node: TSESTree.ImportDeclaration
37+
) {
38+
const fireEventImportNode = node.specifiers.find(
39+
specifier =>
40+
isImportSpecifier(specifier) &&
41+
specifier.imported &&
42+
'fireEvent' === specifier.imported.name
43+
) as TSESTree.ImportSpecifier;
44+
45+
const { references } = context.getDeclaredVariables(
46+
fireEventImportNode
47+
)[0];
48+
49+
for (const reference of references) {
50+
const referenceNode = reference.identifier;
51+
const callExpression = referenceNode.parent
52+
.parent as TSESTree.CallExpression;
53+
const [element] = callExpression.arguments as TSESTree.Node[];
54+
if (isCallExpression(element) || isNewExpression(element)) {
55+
const methodName = isIdentifier(element.callee)
56+
? element.callee.name
57+
: ((element.callee as TSESTree.MemberExpression)
58+
.property as TSESTree.Identifier).name;
59+
60+
if (
61+
ASYNC_QUERIES_VARIANTS.some(q => methodName.startsWith(q)) ||
62+
methodName === 'Promise'
63+
) {
64+
context.report({
65+
node: element,
66+
messageId: 'noPromiseInFireEvent',
67+
});
68+
}
69+
}
70+
}
71+
},
72+
};
73+
},
74+
});

‎tests/__snapshots__/index.test.ts.snap

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Object {
1414
"error",
1515
"angular",
1616
],
17+
"testing-library/no-promise-in-fire-event": "error",
1718
"testing-library/no-wait-for-empty-callback": "error",
1819
"testing-library/prefer-find-by": "error",
1920
"testing-library/prefer-screen-queries": "error",
@@ -35,6 +36,7 @@ Object {
3536
"error",
3637
"react",
3738
],
39+
"testing-library/no-promise-in-fire-event": "error",
3840
"testing-library/no-wait-for-empty-callback": "error",
3941
"testing-library/prefer-find-by": "error",
4042
"testing-library/prefer-screen-queries": "error",
@@ -51,6 +53,7 @@ Object {
5153
"testing-library/await-async-query": "error",
5254
"testing-library/await-async-utils": "error",
5355
"testing-library/no-await-sync-query": "error",
56+
"testing-library/no-promise-in-fire-event": "error",
5457
"testing-library/no-wait-for-empty-callback": "error",
5558
"testing-library/prefer-find-by": "error",
5659
"testing-library/prefer-screen-queries": "error",
@@ -73,6 +76,7 @@ Object {
7376
"error",
7477
"vue",
7578
],
79+
"testing-library/no-promise-in-fire-event": "error",
7680
"testing-library/no-wait-for-empty-callback": "error",
7781
"testing-library/prefer-find-by": "error",
7882
"testing-library/prefer-screen-queries": "error",

‎tests/lib/rules/no-promise-in-fire-event.test.ts

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { createRuleTester } from '../test-utils';
2+
import rule, { RULE_NAME } from '../../../lib/rules/no-promise-in-fire-event';
3+
4+
const ruleTester = createRuleTester();
5+
6+
ruleTester.run(RULE_NAME, rule, {
7+
valid: [
8+
{
9+
code: `
10+
import {fireEvent} from '@testing-library/foo';
11+
12+
fireEvent.click(screen.getByRole('button'))
13+
`,
14+
},
15+
{
16+
code: `
17+
import {fireEvent} from '@testing-library/foo';
18+
19+
fireEvent.click(queryByRole('button'))`,
20+
},
21+
{
22+
code: `
23+
import {fireEvent} from '@testing-library/foo';
24+
25+
fireEvent.click(someRef)`,
26+
},
27+
{
28+
code: `fireEvent.click(findByText('submit'))`,
29+
},
30+
{
31+
code: `
32+
import {fireEvent} from '@testing-library/foo';
33+
34+
const promise = new Promise();
35+
fireEvent.click(promise)`,
36+
},
37+
{
38+
code: `
39+
import {fireEvent} from '@testing-library/foo';
40+
41+
fireEvent.click(await screen.findByRole('button'))
42+
`,
43+
},
44+
{
45+
code: `fireEvent.click(Promise())`,
46+
},
47+
],
48+
invalid: [
49+
{
50+
code: `
51+
import {fireEvent} from '@testing-library/foo';
52+
53+
fireEvent.click(screen.findByRole('button'))`,
54+
errors: [
55+
{
56+
messageId: 'noPromiseInFireEvent',
57+
},
58+
],
59+
},
60+
{
61+
code: `
62+
import {fireEvent} from '@testing-library/foo';
63+
64+
fireEvent.click(findByText('submit'))`,
65+
errors: [
66+
{
67+
messageId: 'noPromiseInFireEvent',
68+
},
69+
],
70+
},
71+
{
72+
code: `
73+
import {fireEvent} from '@testing-library/foo';
74+
75+
fireEvent.click(Promise('foo'))`,
76+
errors: [
77+
{
78+
messageId: 'noPromiseInFireEvent',
79+
},
80+
],
81+
},
82+
{
83+
code: `
84+
import {fireEvent} from '@testing-library/foo';
85+
86+
fireEvent.click(new Promise('foo'))`,
87+
errors: [
88+
{
89+
messageId: 'noPromiseInFireEvent',
90+
},
91+
],
92+
},
93+
],
94+
});

0 commit comments

Comments
 (0)