Skip to content

Commit 57de2b3

Browse files
committed
feat(no-promise-in-fire-event): add new no-promise-in-fire-event rule
1 parent edf9e66 commit 57de2b3

File tree

5 files changed

+220
-3
lines changed

5 files changed

+220
-3
lines changed
+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
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+
29+
## Further Reading
30+
31+
- [A Github Issue explaining the problem](https://github.com/testing-library/dom-testing-library/issues/609)

lib/index.ts

+2
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,

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

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

0 commit comments

Comments
 (0)