Skip to content

Commit 7e9d5c5

Browse files
authored
fix: first round of bugfixes for v4
Closes #307. Closes #304
2 parents 82ace49 + c801b1d commit 7e9d5c5

12 files changed

+288
-113
lines changed

lib/detect-testing-library-utils.ts

+14-6
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,20 @@ export function detectTestingLibraryUtils<
335335
* Not to be confused with {@link isUserEventMethod}
336336
*/
337337
const isUserEventUtil = (node: TSESTree.Identifier): boolean => {
338-
return isTestingLibraryUtil(
339-
node,
340-
(identifierNodeName, originalNodeName) => {
341-
return [identifierNodeName, originalNodeName].includes('userEvent');
342-
}
343-
);
338+
const userEvent = findImportedUserEventSpecifier();
339+
let userEventName: string | undefined;
340+
341+
if (userEvent) {
342+
userEventName = userEvent.name;
343+
} else if (isAggressiveModuleReportingEnabled()) {
344+
userEventName = USER_EVENT_NAME;
345+
}
346+
347+
if (!userEventName) {
348+
return false;
349+
}
350+
351+
return node.name === userEventName;
344352
};
345353

346354
/**

lib/node-utils.ts

-13
Original file line numberDiff line numberDiff line change
@@ -153,19 +153,6 @@ export function findClosestCallNode(
153153
}
154154
}
155155

156-
export function isCallExpressionCallee(
157-
node: TSESTree.CallExpression,
158-
identifier: TSESTree.Identifier
159-
): boolean {
160-
const nodeInnerIdentifier = getDeepestIdentifierNode(node);
161-
162-
if (nodeInnerIdentifier) {
163-
return nodeInnerIdentifier.name === identifier.name;
164-
}
165-
166-
return false;
167-
}
168-
169156
export function isObjectExpression(
170157
node: TSESTree.Expression
171158
): node is TSESTree.ObjectExpression {

lib/rules/await-async-utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
8080
node,
8181
messageId: 'awaitAsyncUtil',
8282
data: {
83-
name: referenceNode.name,
83+
name: node.name,
8484
},
8585
});
8686
}

lib/rules/no-await-sync-query.ts

+7-13
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import { TSESTree } from '@typescript-eslint/experimental-utils';
22
import { createTestingLibraryRule } from '../create-testing-library-rule';
3-
import {
4-
findClosestCallExpressionNode,
5-
isCallExpressionCallee,
6-
} from '../node-utils';
3+
import { getDeepestIdentifierNode } from '../node-utils';
74

85
export const RULE_NAME = 'no-await-sync-query';
96
export type MessageIds = 'noAwaitSyncQuery';
@@ -29,22 +26,19 @@ export default createTestingLibraryRule<Options, MessageIds>({
2926

3027
create(context, _, helpers) {
3128
return {
32-
'AwaitExpression > CallExpression Identifier'(node: TSESTree.Identifier) {
33-
const closestCallExpression = findClosestCallExpressionNode(node, true);
34-
if (!closestCallExpression) {
35-
return;
36-
}
29+
'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) {
30+
const deepestIdentifierNode = getDeepestIdentifierNode(node);
3731

38-
if (!isCallExpressionCallee(closestCallExpression, node)) {
32+
if (!deepestIdentifierNode) {
3933
return;
4034
}
4135

42-
if (helpers.isSyncQuery(node)) {
36+
if (helpers.isSyncQuery(deepestIdentifierNode)) {
4337
context.report({
44-
node,
38+
node: deepestIdentifierNode,
4539
messageId: 'noAwaitSyncQuery',
4640
data: {
47-
name: node.name,
41+
name: deepestIdentifierNode.name,
4842
},
4943
});
5044
}

lib/rules/no-wait-for-multiple-assertions.ts

+24-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { TSESTree } from '@typescript-eslint/experimental-utils';
2-
import { getPropertyIdentifierNode } from '../node-utils';
2+
import {
3+
getPropertyIdentifierNode,
4+
isExpressionStatement,
5+
} from '../node-utils';
36
import { createTestingLibraryRule } from '../create-testing-library-rule';
47

58
export const RULE_NAME = 'no-wait-for-multiple-assertions';
@@ -24,16 +27,21 @@ export default createTestingLibraryRule<Options, MessageIds>({
2427
},
2528
defaultOptions: [],
2629
create: function (context, _, helpers) {
27-
function totalExpect(body: Array<TSESTree.Node>): Array<TSESTree.Node> {
28-
return body.filter((node: TSESTree.ExpressionStatement) => {
29-
const expressionIdentifier = getPropertyIdentifierNode(node);
30+
function getExpectNodes(
31+
body: Array<TSESTree.Node>
32+
): Array<TSESTree.ExpressionStatement> {
33+
return body.filter((node) => {
34+
if (!isExpressionStatement(node)) {
35+
return false;
36+
}
3037

38+
const expressionIdentifier = getPropertyIdentifierNode(node);
3139
if (!expressionIdentifier) {
3240
return false;
3341
}
3442

3543
return expressionIdentifier.name === 'expect';
36-
});
44+
}) as Array<TSESTree.ExpressionStatement>;
3745
}
3846

3947
function reportMultipleAssertion(node: TSESTree.BlockStatement) {
@@ -46,14 +54,20 @@ export default createTestingLibraryRule<Options, MessageIds>({
4654
return;
4755
}
4856

49-
if (totalExpect(node.body).length <= 1) {
57+
const expectNodes = getExpectNodes(node.body);
58+
59+
if (expectNodes.length <= 1) {
5060
return;
5161
}
5262

53-
context.report({
54-
node: callExpressionNode,
55-
messageId: 'noWaitForMultipleAssertion',
56-
});
63+
for (let i = 0; i < expectNodes.length; i++) {
64+
if (i !== 0) {
65+
context.report({
66+
node: expectNodes[i],
67+
messageId: 'noWaitForMultipleAssertion',
68+
});
69+
}
70+
}
5771
}
5872

5973
return {

lib/rules/no-wait-for-side-effects.ts

+21-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { TSESTree } from '@typescript-eslint/experimental-utils';
2-
import { getPropertyIdentifierNode } from '../node-utils';
2+
import {
3+
getPropertyIdentifierNode,
4+
isExpressionStatement,
5+
} from '../node-utils';
36
import { createTestingLibraryRule } from '../create-testing-library-rule';
47

58
export const RULE_NAME = 'no-wait-for-side-effects';
@@ -24,10 +27,15 @@ export default createTestingLibraryRule<Options, MessageIds>({
2427
},
2528
defaultOptions: [],
2629
create: function (context, _, helpers) {
27-
function hasSideEffects(body: Array<TSESTree.Node>): boolean {
28-
return body.some((node: TSESTree.ExpressionStatement) => {
29-
const expressionIdentifier = getPropertyIdentifierNode(node);
30+
function getSideEffectNodes(
31+
body: TSESTree.Node[]
32+
): TSESTree.ExpressionStatement[] {
33+
return body.filter((node) => {
34+
if (!isExpressionStatement(node)) {
35+
return false;
36+
}
3037

38+
const expressionIdentifier = getPropertyIdentifierNode(node);
3139
if (!expressionIdentifier) {
3240
return false;
3341
}
@@ -36,7 +44,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
3644
helpers.isFireEventUtil(expressionIdentifier) ||
3745
helpers.isUserEventUtil(expressionIdentifier)
3846
);
39-
});
47+
}) as TSESTree.ExpressionStatement[];
4048
}
4149

4250
function reportSideEffects(node: TSESTree.BlockStatement) {
@@ -49,14 +57,17 @@ export default createTestingLibraryRule<Options, MessageIds>({
4957
return;
5058
}
5159

52-
if (!hasSideEffects(node.body)) {
60+
const sideEffectNodes = getSideEffectNodes(node.body);
61+
if (sideEffectNodes.length === 0) {
5362
return;
5463
}
5564

56-
context.report({
57-
node: callExpressionNode,
58-
messageId: 'noSideEffectsWaitFor',
59-
});
65+
for (const sideEffectNode of sideEffectNodes) {
66+
context.report({
67+
node: sideEffectNode,
68+
messageId: 'noSideEffectsWaitFor',
69+
});
70+
}
6071
}
6172

6273
return {

lib/rules/prefer-find-by.ts

+30-14
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export const RULE_NAME = 'prefer-find-by';
1717
export type MessageIds = 'preferFindBy';
1818
type Options = [];
1919

20-
export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait'];
20+
export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait'] as const;
2121

2222
export function getFindByQueryVariant(
2323
queryMethod: string
@@ -53,13 +53,13 @@ export default createTestingLibraryRule<Options, MessageIds>({
5353
type: 'suggestion',
5454
docs: {
5555
description:
56-
'Suggest using find* instead of waitFor to wait for elements',
56+
'Suggest using `find*` query instead of `waitFor` + `get*` to wait for elements',
5757
category: 'Best Practices',
5858
recommended: 'warn',
5959
},
6060
messages: {
6161
preferFindBy:
62-
'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}',
62+
'Prefer `{{queryVariant}}{{queryMethod}}` query over using `{{waitForMethodName}}` + `{{prevQuery}}`',
6363
},
6464
fixable: 'code',
6565
schema: [],
@@ -71,30 +71,39 @@ export default createTestingLibraryRule<Options, MessageIds>({
7171

7272
/**
7373
* Reports the invalid usage of wait* plus getBy/QueryBy methods and automatically fixes the scenario
74-
* @param {TSESTree.CallExpression} node - The CallExpresion node that contains the wait* method
75-
* @param {'findBy' | 'findAllBy'} replacementParams.queryVariant - The variant method used to query: findBy/findByAll.
76-
* @param {string} replacementParams.queryMethod - Suffix string to build the query method (the query-part that comes after the "By"): LabelText, Placeholder, Text, Role, Title, etc.
77-
* @param {ReportFixFunction} replacementParams.fix - Function that applies the fix to correct the code
74+
* @param node - The CallExpresion node that contains the wait* method
75+
* @param replacementParams - Object with info for error message and autofix:
76+
* @param replacementParams.queryVariant - The variant method used to query: findBy/findAllBy.
77+
* @param replacementParams.prevQuery - The query originally used inside `waitFor`
78+
* @param replacementParams.queryMethod - Suffix string to build the query method (the query-part that comes after the "By"): LabelText, Placeholder, Text, Role, Title, etc.
79+
* @param replacementParams.waitForMethodName - wait for method used: waitFor/wait/waitForElement
80+
* @param replacementParams.fix - Function that applies the fix to correct the code
7881
*/
7982
function reportInvalidUsage(
8083
node: TSESTree.CallExpression,
81-
{
82-
queryVariant,
83-
queryMethod,
84-
fix,
85-
}: {
84+
replacementParams: {
8685
queryVariant: 'findBy' | 'findAllBy';
8786
queryMethod: string;
87+
prevQuery: string;
88+
waitForMethodName: string;
8889
fix: ReportFixFunction;
8990
}
9091
) {
92+
const {
93+
queryMethod,
94+
queryVariant,
95+
prevQuery,
96+
waitForMethodName,
97+
fix,
98+
} = replacementParams;
9199
context.report({
92100
node,
93101
messageId: 'preferFindBy',
94102
data: {
95103
queryVariant,
96104
queryMethod,
97-
fullQuery: sourceCode.getText(node),
105+
prevQuery,
106+
waitForMethodName,
98107
},
99108
fix,
100109
});
@@ -104,7 +113,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
104113
'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) {
105114
if (
106115
!ASTUtils.isIdentifier(node.callee) ||
107-
!WAIT_METHODS.includes(node.callee.name)
116+
!helpers.isAsyncUtil(node.callee, WAIT_METHODS)
108117
) {
109118
return;
110119
}
@@ -117,6 +126,9 @@ export default createTestingLibraryRule<Options, MessageIds>({
117126
if (!isCallExpression(argument.body)) {
118127
return;
119128
}
129+
130+
const waitForMethodName = node.callee.name;
131+
120132
// ensure here it's one of the sync methods that we are calling
121133
if (
122134
isMemberExpression(argument.body.callee) &&
@@ -134,6 +146,8 @@ export default createTestingLibraryRule<Options, MessageIds>({
134146
reportInvalidUsage(node, {
135147
queryMethod,
136148
queryVariant,
149+
prevQuery: fullQueryMethod,
150+
waitForMethodName,
137151
fix(fixer) {
138152
const property = ((argument.body as TSESTree.CallExpression)
139153
.callee as TSESTree.MemberExpression).property;
@@ -163,6 +177,8 @@ export default createTestingLibraryRule<Options, MessageIds>({
163177
reportInvalidUsage(node, {
164178
queryMethod,
165179
queryVariant,
180+
prevQuery: fullQueryMethod,
181+
waitForMethodName,
166182
fix(fixer) {
167183
// we know from above callee is an Identifier
168184
if (

0 commit comments

Comments
 (0)