Skip to content

Commit 6f506ee

Browse files
Belco90Belco90
authored
refactor(no-promise-in-fire-event): use custom rule creator (#266)
* refactor(await-async-utils): create rule with custom creator * test(await-async-utils): remove outdated comment * docs(no-promise-in-fire-event): improve description and examples * docs(no-promise-in-fire-event): improve invalid errors checks * refactor(no-promise-in-fire-event): use custom rule creator and helpers * feat(no-promise-in-fire-event): detect promise in variable references * docs(no-promise-in-fire-event): update examples * test(no-promise-in-fire-event): increase coverage up to 100%
1 parent 668a1bf commit 6f506ee

File tree

4 files changed

+186
-80
lines changed

4 files changed

+186
-80
lines changed

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,45 @@
11
# Disallow the use of promises passed to a `fireEvent` method (no-promise-in-fire-event)
22

3-
The `fireEvent` method expects that a DOM element is passed.
3+
Methods from `fireEvent` expect to receive a DOM element. Passing a promise will end up in an error, so it must be prevented.
44

55
Examples of **incorrect** code for this rule:
66

77
```js
88
import { screen, fireEvent } from '@testing-library/react';
99

10-
// usage of findBy queries
10+
// usage of unhandled findBy queries
1111
fireEvent.click(screen.findByRole('button'));
1212

13-
// usage of promises
14-
fireEvent.click(new Promise(jest.fn())
13+
// usage of unhandled promises
14+
fireEvent.click(new Promise(jest.fn()));
15+
16+
// usage of references to unhandled promises
17+
const promise = new Promise();
18+
fireEvent.click(promise);
19+
20+
const anotherPromise = screen.findByRole('button');
21+
fireEvent.click(anotherPromise);
1522
```
1623

1724
Examples of **correct** code for this rule:
1825

1926
```js
2027
import { screen, fireEvent } from '@testing-library/react';
2128

22-
// use getBy queries
29+
// usage of getBy queries
2330
fireEvent.click(screen.getByRole('button'));
2431

25-
// use awaited findBy queries
32+
// usage of awaited findBy queries
2633
fireEvent.click(await screen.findByRole('button'));
2734

28-
// this won't give a linting error, but it will throw a runtime error
35+
// usage of references to handled promises
2936
const promise = new Promise();
30-
fireEvent.click(promise)`,
37+
const element = await promise;
38+
fireEvent.click(element);
39+
40+
const anotherPromise = screen.findByRole('button');
41+
const button = await anotherPromise;
42+
fireEvent.click(button);
3143
```
3244

3345
## Further Reading

‎lib/node-utils.ts‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ export function isBlockStatement(
8383
return node?.type === AST_NODE_TYPES.BlockStatement;
8484
}
8585

86-
export function isVariableDeclarator(
87-
node: TSESTree.Node
88-
): node is TSESTree.VariableDeclarator {
89-
return node?.type === AST_NODE_TYPES.VariableDeclarator;
90-
}
91-
9286
export function isObjectPattern(
9387
node: TSESTree.Node
9488
): node is TSESTree.ObjectPattern {
@@ -191,14 +185,6 @@ export function isImportDeclaration(
191185
return node?.type === AST_NODE_TYPES.ImportDeclaration;
192186
}
193187

194-
export function isAwaited(node: TSESTree.Node): boolean {
195-
return (
196-
ASTUtils.isAwaitExpression(node) ||
197-
isArrowFunctionExpression(node) ||
198-
isReturnStatement(node)
199-
);
200-
}
201-
202188
export function hasChainedThen(node: TSESTree.Node): boolean {
203189
const parent = node.parent;
204190

@@ -211,11 +197,16 @@ export function hasChainedThen(node: TSESTree.Node): boolean {
211197
return hasThenProperty(parent);
212198
}
213199

200+
export function isPromiseIdentifier(
201+
node: TSESTree.Node
202+
): node is TSESTree.Identifier & { name: 'Promise' } {
203+
return ASTUtils.isIdentifier(node) && node.name === 'Promise';
204+
}
205+
214206
export function isPromiseAll(node: TSESTree.CallExpression): boolean {
215207
return (
216208
isMemberExpression(node.callee) &&
217-
ASTUtils.isIdentifier(node.callee.object) &&
218-
node.callee.object.name === 'Promise' &&
209+
isPromiseIdentifier(node.callee.object) &&
219210
ASTUtils.isIdentifier(node.callee.property) &&
220211
node.callee.property.name === 'all'
221212
);
@@ -224,8 +215,7 @@ export function isPromiseAll(node: TSESTree.CallExpression): boolean {
224215
export function isPromiseAllSettled(node: TSESTree.CallExpression): boolean {
225216
return (
226217
isMemberExpression(node.callee) &&
227-
ASTUtils.isIdentifier(node.callee.object) &&
228-
node.callee.object.name === 'Promise' &&
218+
isPromiseIdentifier(node.callee.object) &&
229219
ASTUtils.isIdentifier(node.callee.property) &&
230220
node.callee.property.name === 'allSettled'
231221
);
@@ -304,7 +294,7 @@ export function getVariableReferences(
304294
node: TSESTree.Node
305295
): TSESLint.Scope.Reference[] {
306296
return (
307-
(isVariableDeclarator(node) &&
297+
(ASTUtils.isVariableDeclarator(node) &&
308298
context.getDeclaredVariables(node)[0]?.references?.slice(1)) ||
309299
[]
310300
);

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

Lines changed: 67 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,18 @@
1+
import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { createTestingLibraryRule } from '../create-testing-library-rule';
13
import {
2-
TSESTree,
3-
ESLintUtils,
4-
ASTUtils,
5-
} from '@typescript-eslint/experimental-utils';
6-
import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils';
7-
import {
8-
isNewExpression,
9-
isImportSpecifier,
4+
findClosestCallExpressionNode,
5+
getIdentifierNode,
106
isCallExpression,
7+
isNewExpression,
8+
isPromiseIdentifier,
119
} from '../node-utils';
1210

1311
export const RULE_NAME = 'no-promise-in-fire-event';
1412
export type MessageIds = 'noPromiseInFireEvent';
1513
type Options = [];
1614

17-
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
15+
export default createTestingLibraryRule<Options, MessageIds>({
1816
name: RULE_NAME,
1917
meta: {
2018
type: 'problem',
@@ -28,49 +26,74 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
2826
noPromiseInFireEvent:
2927
"A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element",
3028
},
31-
fixable: 'code',
29+
fixable: null,
3230
schema: [],
3331
},
3432
defaultOptions: [],
3533

36-
create(context) {
37-
return {
38-
'ImportDeclaration[source.value=/testing-library/]'(
39-
node: TSESTree.ImportDeclaration
40-
) {
41-
const fireEventImportNode = node.specifiers.find(
42-
(specifier) =>
43-
isImportSpecifier(specifier) &&
44-
specifier.imported &&
45-
'fireEvent' === specifier.imported.name
46-
) as TSESTree.ImportSpecifier;
34+
create(context, _, helpers) {
35+
function checkSuspiciousNode(
36+
node: TSESTree.Node,
37+
originalNode?: TSESTree.Node
38+
): void {
39+
if (ASTUtils.isAwaitExpression(node)) {
40+
return;
41+
}
4742

48-
const { references } = context.getDeclaredVariables(
49-
fireEventImportNode
50-
)[0];
43+
if (isNewExpression(node)) {
44+
if (isPromiseIdentifier(node.callee)) {
45+
return context.report({
46+
node: originalNode ?? node,
47+
messageId: 'noPromiseInFireEvent',
48+
});
49+
}
50+
}
5151

52-
for (const reference of references) {
53-
const referenceNode = reference.identifier;
54-
const callExpression = referenceNode.parent
55-
.parent as TSESTree.CallExpression;
56-
const [element] = callExpression.arguments as TSESTree.Node[];
57-
if (isCallExpression(element) || isNewExpression(element)) {
58-
const methodName = ASTUtils.isIdentifier(element.callee)
59-
? element.callee.name
60-
: ((element.callee as TSESTree.MemberExpression)
61-
.property as TSESTree.Identifier).name;
52+
if (isCallExpression(node)) {
53+
const domElementIdentifier = getIdentifierNode(node);
54+
55+
if (
56+
helpers.isAsyncQuery(domElementIdentifier) ||
57+
isPromiseIdentifier(domElementIdentifier)
58+
) {
59+
return context.report({
60+
node: originalNode ?? node,
61+
messageId: 'noPromiseInFireEvent',
62+
});
63+
}
64+
}
65+
66+
if (ASTUtils.isIdentifier(node)) {
67+
const nodeVariable = ASTUtils.findVariable(
68+
context.getScope(),
69+
node.name
70+
);
71+
if (!nodeVariable || !nodeVariable.defs) {
72+
return;
73+
}
6274

63-
if (
64-
ASYNC_QUERIES_VARIANTS.some((q) => methodName.startsWith(q)) ||
65-
methodName === 'Promise'
66-
) {
67-
context.report({
68-
node: element,
69-
messageId: 'noPromiseInFireEvent',
70-
});
71-
}
72-
}
75+
for (const definition of nodeVariable.defs) {
76+
const variableDeclarator = definition.node as TSESTree.VariableDeclarator;
77+
checkSuspiciousNode(variableDeclarator.init, node);
7378
}
79+
}
80+
}
81+
82+
return {
83+
'CallExpression Identifier'(node: TSESTree.Identifier) {
84+
if (!helpers.isFireEventMethod(node)) {
85+
return;
86+
}
87+
88+
const closestCallExpression = findClosestCallExpressionNode(node, true);
89+
90+
if (!closestCallExpression) {
91+
return;
92+
}
93+
94+
const domElementArgument = closestCallExpression.arguments[0];
95+
96+
checkSuspiciousNode(domElementArgument);
7497
},
7598
};
7699
},

0 commit comments

Comments
 (0)