Skip to content

Commit 668a1bf

Browse files
authored
refactor(await-fire-event): use custom rule creator (#265)
* refactor(await-async-utils): create rule with custom creator * docs(await-fire-event): update description * refactor(await-fire-event): create rule with custom creator * refactor(await-fire-event): replace manual promise checks by util * refactor: simplify isNodeComingFromTestingLibrary * fix: call findClosestCallExpressionNode recursively keeping args * refactor(prefer-user-event): extract fire event helpers * refactor(await-async-utils): remove unnecessary as expression * refactor(await-fire-event): reuse fire event detection helpers * feat(await-fire-event): detect functions wrapping fire event methods * fix(await-fire-event): detect more cases * test(await-fire-event): increase coverage * docs(await-fire-event): update rule details and examples * test(await-async-utils): remove outdated comment * docs(await-fire-event): update async note * style(await-fire-event): format rule doc * refactor(await-fire-event): remove unnecessary check
1 parent 9062040 commit 668a1bf

9 files changed

+568
-190
lines changed

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ To enable this configuration use the `extends` property in your
129129
| -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ |
130130
| [await-async-query](docs/rules/await-async-query.md) | Enforce promises from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
131131
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
132-
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
132+
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce promises from fire event methods to be handled | ![vue-badge][] | |
133133
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
134134
| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | | |
135135
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |

docs/rules/await-fire-event.md

+28-8
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
1-
# Enforce async fire event methods to be awaited (await-fire-event)
1+
# Enforce promises from fire event methods to be handled (await-fire-event)
22

3-
Ensure that promises returned by `fireEvent` methods are awaited
3+
Ensure that promises returned by `fireEvent` methods are handled
44
properly.
55

66
## Rule Details
77

8-
This rule aims to prevent users from forgetting to await `fireEvent`
9-
methods when they are async.
8+
This rule aims to prevent users from forgetting to handle promise returned from `fireEvent`
9+
methods.
10+
11+
> ⚠️ `fireEvent` methods are async only on following Testing Library packages:
12+
>
13+
> - `@testing-library/vue` (supported by this plugin)
14+
> - `@testing-library/svelte` (not supported yet by this plugin)
1015
1116
Examples of **incorrect** code for this rule:
1217

@@ -15,6 +20,12 @@ fireEvent.click(getByText('Click me'));
1520

1621
fireEvent.focus(getByLabelText('username'));
1722
fireEvent.blur(getByLabelText('username'));
23+
24+
// wrap a fireEvent method within a function...
25+
function triggerEvent() {
26+
return fireEvent.click(button);
27+
}
28+
triggerEvent(); // ...but not handling promise from it is incorrect too
1829
```
1930

2031
Examples of **correct** code for this rule:
@@ -30,15 +41,24 @@ fireEvent.click(getByText('Click me')).then(() => {
3041
});
3142

3243
// return the promise within a function is correct too!
33-
function clickMeRegularFn() {
34-
return fireEvent.click(getByText('Click me'));
35-
}
3644
const clickMeArrowFn = () => fireEvent.click(getByText('Click me'));
45+
46+
// wrap a fireEvent method within a function...
47+
function triggerEvent() {
48+
return fireEvent.click(button);
49+
}
50+
await triggerEvent(); // ...and handling promise from it is correct also
51+
52+
// using `Promise.all` or `Promise.allSettled` with an array of promises is valid
53+
await Promise.all([
54+
fireEvent.focus(getByLabelText('username')),
55+
fireEvent.blur(getByLabelText('username')),
56+
]);
3757
```
3858

3959
## When Not To Use It
4060

41-
`fireEvent` methods are only async in Vue Testing Library so if you are using another Testing Library module, you shouldn't use this rule.
61+
`fireEvent` methods are not async on all Testing Library packages. If you are not using Testing Library package with async fire event, you shouldn't use this rule.
4262

4363
## Further Reading
4464

lib/detect-testing-library-utils.ts

+83-46
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,15 @@ import {
44
TSESTree,
55
} from '@typescript-eslint/experimental-utils';
66
import {
7-
getImportModuleName,
87
getAssertNodeInfo,
9-
isLiteral,
8+
getImportModuleName,
109
ImportModuleNode,
1110
isImportDeclaration,
1211
isImportNamespaceSpecifier,
1312
isImportSpecifier,
13+
isLiteral,
14+
isMemberExpression,
1415
isProperty,
15-
isCallExpression,
16-
isObjectPattern,
1716
} from './node-utils';
1817
import { ABSENCE_MATCHERS, ASYNC_UTILS, PRESENCE_MATCHERS } from './utils';
1918

@@ -54,6 +53,7 @@ export type DetectionHelpers = {
5453
isSyncQuery: (node: TSESTree.Identifier) => boolean;
5554
isAsyncQuery: (node: TSESTree.Identifier) => boolean;
5655
isAsyncUtil: (node: TSESTree.Identifier) => boolean;
56+
isFireEventMethod: (node: TSESTree.Identifier) => boolean;
5757
isPresenceAssert: (node: TSESTree.MemberExpression) => boolean;
5858
isAbsenceAssert: (node: TSESTree.MemberExpression) => boolean;
5959
canReportErrors: () => boolean;
@@ -67,6 +67,8 @@ export type DetectionHelpers = {
6767

6868
const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';
6969

70+
const FIRE_EVENT_NAME = 'fireEvent';
71+
7072
/**
7173
* Enhances a given rule `create` with helpers to detect Testing Library utils.
7274
*/
@@ -88,6 +90,15 @@ export function detectTestingLibraryUtils<
8890
context.settings['testing-library/filename-pattern'] ??
8991
DEFAULT_FILENAME_PATTERN;
9092

93+
/**
94+
* Determines whether aggressive reporting is enabled or not.
95+
*
96+
* Aggressive reporting is considered as enabled when:
97+
* - custom module is not set (so we need to assume everything
98+
* matching TL utils is related to TL no matter where it was imported from)
99+
*/
100+
const isAggressiveReportingEnabled = () => !customModule;
101+
91102
// Helpers for Testing Library detection.
92103
const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => {
93104
return importedTestingLibraryNode;
@@ -118,7 +129,7 @@ export function detectTestingLibraryUtils<
118129
* or custom module are imported.
119130
*/
120131
const isTestingLibraryImported: DetectionHelpers['isTestingLibraryImported'] = () => {
121-
if (!customModule) {
132+
if (isAggressiveReportingEnabled()) {
122133
return true;
123134
}
124135

@@ -176,6 +187,58 @@ export function detectTestingLibraryUtils<
176187
return ASYNC_UTILS.includes(node.name);
177188
};
178189

190+
/**
191+
* Determines whether a given node is fireEvent method or not
192+
*/
193+
const isFireEventMethod: DetectionHelpers['isFireEventMethod'] = (node) => {
194+
const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME);
195+
let fireEventUtilName: string | undefined;
196+
197+
if (fireEventUtil) {
198+
fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil)
199+
? fireEventUtil.name
200+
: fireEventUtil.local.name;
201+
} else if (isAggressiveReportingEnabled()) {
202+
fireEventUtilName = FIRE_EVENT_NAME;
203+
}
204+
205+
if (!fireEventUtilName) {
206+
return false;
207+
}
208+
209+
const parentMemberExpression:
210+
| TSESTree.MemberExpression
211+
| undefined = isMemberExpression(node.parent) ? node.parent : undefined;
212+
213+
if (!parentMemberExpression) {
214+
return false;
215+
}
216+
217+
// make sure that given node it's not fireEvent object itself
218+
if (
219+
[fireEventUtilName, FIRE_EVENT_NAME].includes(node.name) ||
220+
(ASTUtils.isIdentifier(parentMemberExpression.object) &&
221+
parentMemberExpression.object.name === node.name)
222+
) {
223+
return false;
224+
}
225+
226+
// check fireEvent.click() usage
227+
const regularCall =
228+
ASTUtils.isIdentifier(parentMemberExpression.object) &&
229+
parentMemberExpression.object.name === fireEventUtilName;
230+
231+
// check testingLibraryUtils.fireEvent.click() usage
232+
const wildcardCall =
233+
isMemberExpression(parentMemberExpression.object) &&
234+
ASTUtils.isIdentifier(parentMemberExpression.object.object) &&
235+
parentMemberExpression.object.object.name === fireEventUtilName &&
236+
ASTUtils.isIdentifier(parentMemberExpression.object.property) &&
237+
parentMemberExpression.object.property.name === FIRE_EVENT_NAME;
238+
239+
return regularCall || wildcardCall;
240+
};
241+
179242
/**
180243
* Determines whether a given MemberExpression node is a presence assert
181244
*
@@ -215,7 +278,8 @@ export function detectTestingLibraryUtils<
215278
};
216279

217280
/**
218-
* Gets a string and verifies if it was imported/required by our custom module node
281+
* Gets a string and verifies if it was imported/required by Testing Library
282+
* related module.
219283
*/
220284
const findImportedUtilSpecifier: DetectionHelpers['findImportedUtilSpecifier'] = (
221285
specifierName
@@ -260,52 +324,24 @@ export function detectTestingLibraryUtils<
260324
};
261325
/**
262326
* Takes a MemberExpression or an Identifier and verifies if its name comes from the import in TL
263-
* @param node a MemberExpression (in "foo.property" it would be property) or an Identifier (it should be provided from a CallExpression, for example "foo()")
327+
* @param node a MemberExpression (in "foo.property" it would be property) or an Identifier
264328
*/
265329
const isNodeComingFromTestingLibrary: DetectionHelpers['isNodeComingFromTestingLibrary'] = (
266330
node: TSESTree.MemberExpression | TSESTree.Identifier
267331
) => {
268-
const importOrRequire =
269-
getCustomModuleImportNode() ?? getTestingLibraryImportNode();
270-
if (!importOrRequire) {
271-
return false;
272-
}
332+
let identifierName: string | undefined;
333+
273334
if (ASTUtils.isIdentifier(node)) {
274-
if (isImportDeclaration(importOrRequire)) {
275-
return importOrRequire.specifiers.some(
276-
(s) => isImportSpecifier(s) && s.local.name === node.name
277-
);
278-
} else {
279-
return (
280-
ASTUtils.isVariableDeclarator(importOrRequire.parent) &&
281-
isObjectPattern(importOrRequire.parent.id) &&
282-
importOrRequire.parent.id.properties.some(
283-
(p) =>
284-
isProperty(p) &&
285-
ASTUtils.isIdentifier(p.key) &&
286-
ASTUtils.isIdentifier(p.value) &&
287-
p.value.name === node.name
288-
)
289-
);
290-
}
291-
} else {
292-
if (!ASTUtils.isIdentifier(node.object)) {
293-
return false;
294-
}
295-
if (isImportDeclaration(importOrRequire)) {
296-
return (
297-
isImportDeclaration(importOrRequire) &&
298-
isImportNamespaceSpecifier(importOrRequire.specifiers[0]) &&
299-
node.object.name === importOrRequire.specifiers[0].local.name
300-
);
301-
}
302-
return (
303-
isCallExpression(importOrRequire) &&
304-
ASTUtils.isVariableDeclarator(importOrRequire.parent) &&
305-
ASTUtils.isIdentifier(importOrRequire.parent.id) &&
306-
node.object.name === importOrRequire.parent.id.name
307-
);
335+
identifierName = node.name;
336+
} else if (ASTUtils.isIdentifier(node.object)) {
337+
identifierName = node.object.name;
338+
}
339+
340+
if (!identifierName) {
341+
return;
308342
}
343+
344+
return !!findImportedUtilSpecifier(identifierName);
309345
};
310346

311347
const helpers = {
@@ -321,6 +357,7 @@ export function detectTestingLibraryUtils<
321357
isSyncQuery,
322358
isAsyncQuery,
323359
isAsyncUtil,
360+
isFireEventMethod,
324361
isPresenceAssert,
325362
isAbsenceAssert,
326363
canReportErrors,

lib/node-utils.ts

+15-9
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ const ValidLeftHandSideExpressions = [
2727
AST_NODE_TYPES.ObjectExpression,
2828
AST_NODE_TYPES.ObjectPattern,
2929
AST_NODE_TYPES.Super,
30-
AST_NODE_TYPES.TemplateLiteral,
3130
AST_NODE_TYPES.ThisExpression,
3231
AST_NODE_TYPES.TSNullKeyword,
3332
AST_NODE_TYPES.TaggedTemplateExpression,
@@ -132,7 +131,7 @@ export function findClosestCallExpressionNode(
132131
return null;
133132
}
134133

135-
return findClosestCallExpressionNode(node.parent);
134+
return findClosestCallExpressionNode(node.parent, shouldRestrictInnerScope);
136135
}
137136

138137
export function findClosestCallNode(
@@ -232,15 +231,22 @@ export function isPromiseAllSettled(node: TSESTree.CallExpression): boolean {
232231
);
233232
}
234233

234+
/**
235+
* Determines whether a given node belongs to handled Promise.all or Promise.allSettled
236+
* array expression.
237+
*/
235238
export function isPromisesArrayResolved(node: TSESTree.Node): boolean {
236-
const parent = node.parent;
239+
const closestCallExpression = findClosestCallExpressionNode(node, true);
240+
241+
if (!closestCallExpression) {
242+
return false;
243+
}
237244

238245
return (
239-
isCallExpression(parent) &&
240-
isArrayExpression(parent.parent) &&
241-
isCallExpression(parent.parent.parent) &&
242-
(isPromiseAll(parent.parent.parent) ||
243-
isPromiseAllSettled(parent.parent.parent))
246+
isArrayExpression(closestCallExpression.parent) &&
247+
isCallExpression(closestCallExpression.parent.parent) &&
248+
(isPromiseAll(closestCallExpression.parent.parent) ||
249+
isPromiseAllSettled(closestCallExpression.parent.parent))
244250
);
245251
}
246252

@@ -253,7 +259,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean {
253259
* - it belongs to the `Promise.allSettled` method
254260
* - it's chained with the `then` method
255261
* - it's returned from a function
256-
* - has `resolves` or `rejects`
262+
* - has `resolves` or `rejects` jest methods
257263
*/
258264
export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean {
259265
const closestCallExpressionNode = findClosestCallExpressionNode(

lib/rules/await-async-utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
7474
);
7575

7676
if (references && references.length === 0) {
77-
if (!isPromiseHandled(node as TSESTree.Identifier)) {
77+
if (!isPromiseHandled(node)) {
7878
return context.report({
7979
node,
8080
messageId: 'awaitAsyncUtil',

0 commit comments

Comments
 (0)