Skip to content

Commit 9f5de30

Browse files
authored
feat(no-wait-for-side-effects): report render usage in waitFor (#363)
* feat(no-wait-for-side-effects): report render in waitFor * refactor: move null check into isTestingLibraryUtil * refactor: move null check into assignment * refactor: add brackets around null check * refactor: introduce early returns in getSideEffectNodes() * refactor: remove newlines * refactor: move checks to separate functions to improve readability * refactor: merge multiple if statements into one * refactor: remove null types in utils * refactor: merge two functions into one, add missing test case * feat: add docs entry * refactor: pass function arg without wrapping it
1 parent fe7c394 commit 9f5de30

File tree

4 files changed

+453
-8
lines changed

4 files changed

+453
-8
lines changed

docs/rules/no-wait-for-side-effects.md

+25-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Rule Details
44

5-
This rule aims to avoid the usage of side effects actions (`fireEvent` or `userEvent`) inside `waitFor`.
5+
This rule aims to avoid the usage of side effects actions (`fireEvent`, `userEvent` or `render`) inside `waitFor`.
66
Since `waitFor` is intended for things that have a non-deterministic amount of time between the action you performed and the assertion passing,
77
the callback can be called (or checked for errors) a non-deterministic number of times and frequency.
88
This will make your side-effect run multiple times.
@@ -32,6 +32,18 @@ Example of **incorrect** code for this rule:
3232
userEvent.click(button);
3333
expect(b).toEqual('b');
3434
});
35+
36+
// or
37+
await waitFor(() => {
38+
render(<App />)
39+
expect(b).toEqual('b');
40+
});
41+
42+
// or
43+
await waitFor(function() {
44+
render(<App />)
45+
expect(b).toEqual('b');
46+
});
3547
};
3648
```
3749

@@ -60,6 +72,18 @@ Examples of **correct** code for this rule:
6072
await waitFor(function() {
6173
expect(b).toEqual('b');
6274
});
75+
76+
// or
77+
render(<App />)
78+
await waitFor(() => {
79+
expect(b).toEqual('b');
80+
});
81+
82+
// or
83+
render(<App />)
84+
await waitFor(function() {
85+
expect(b).toEqual('b');
86+
});
6387
};
6488
```
6589

lib/node-utils/is-node-of-type.ts

+9
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ export const isCallExpression = isNodeOfType(AST_NODE_TYPES.CallExpression);
1616
export const isExpressionStatement = isNodeOfType(
1717
AST_NODE_TYPES.ExpressionStatement
1818
);
19+
export const isVariableDeclaration = isNodeOfType(
20+
AST_NODE_TYPES.VariableDeclaration
21+
);
22+
export const isAssignmentExpression = isNodeOfType(
23+
AST_NODE_TYPES.AssignmentExpression
24+
);
25+
export const isSequenceExpression = isNodeOfType(
26+
AST_NODE_TYPES.SequenceExpression
27+
);
1928
export const isImportDeclaration = isNodeOfType(
2029
AST_NODE_TYPES.ImportDeclaration
2130
);

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

+87-7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ import { TSESTree } from '@typescript-eslint/experimental-utils';
22
import {
33
getPropertyIdentifierNode,
44
isExpressionStatement,
5+
isVariableDeclaration,
6+
isAssignmentExpression,
7+
isCallExpression,
8+
isSequenceExpression,
59
} from '../node-utils';
610
import { createTestingLibraryRule } from '../create-testing-library-rule';
711

@@ -32,7 +36,11 @@ export default createTestingLibraryRule<Options, MessageIds>({
3236
defaultOptions: [],
3337
create: function (context, _, helpers) {
3438
function isCallerWaitFor(
35-
node: TSESTree.BlockStatement | TSESTree.CallExpression
39+
node:
40+
| TSESTree.BlockStatement
41+
| TSESTree.CallExpression
42+
| TSESTree.AssignmentExpression
43+
| TSESTree.SequenceExpression
3644
): boolean {
3745
if (!node.parent) {
3846
return false;
@@ -48,22 +56,78 @@ export default createTestingLibraryRule<Options, MessageIds>({
4856
);
4957
}
5058

59+
function isRenderInVariableDeclaration(node: TSESTree.Node) {
60+
return (
61+
isVariableDeclaration(node) &&
62+
node.declarations.some(helpers.isRenderVariableDeclarator)
63+
);
64+
}
65+
66+
function isRenderInExpressionStatement(node: TSESTree.Node) {
67+
if (
68+
!isExpressionStatement(node) ||
69+
!isAssignmentExpression(node.expression)
70+
) {
71+
return false;
72+
}
73+
74+
const expressionIdentifier = getPropertyIdentifierNode(
75+
node.expression.right
76+
);
77+
78+
if (!expressionIdentifier) {
79+
return false;
80+
}
81+
82+
return helpers.isRenderUtil(expressionIdentifier);
83+
}
84+
85+
function isRenderInAssignmentExpression(node: TSESTree.Node) {
86+
if (!isAssignmentExpression(node)) {
87+
return false;
88+
}
89+
90+
const expressionIdentifier = getPropertyIdentifierNode(node.right);
91+
if (!expressionIdentifier) {
92+
return false;
93+
}
94+
95+
return helpers.isRenderUtil(expressionIdentifier);
96+
}
97+
98+
function isRenderInSequenceAssignment(node: TSESTree.Node) {
99+
if (!isSequenceExpression(node)) {
100+
return false;
101+
}
102+
103+
return node.expressions.some(isRenderInAssignmentExpression);
104+
}
105+
51106
function getSideEffectNodes(
52107
body: TSESTree.Node[]
53108
): TSESTree.ExpressionStatement[] {
54109
return body.filter((node) => {
55-
if (!isExpressionStatement(node)) {
110+
if (!isExpressionStatement(node) && !isVariableDeclaration(node)) {
56111
return false;
57112
}
58113

114+
if (
115+
isRenderInVariableDeclaration(node) ||
116+
isRenderInExpressionStatement(node)
117+
) {
118+
return true;
119+
}
120+
59121
const expressionIdentifier = getPropertyIdentifierNode(node);
122+
60123
if (!expressionIdentifier) {
61124
return false;
62125
}
63126

64127
return (
65128
helpers.isFireEventUtil(expressionIdentifier) ||
66-
helpers.isUserEventUtil(expressionIdentifier)
129+
helpers.isUserEventUtil(expressionIdentifier) ||
130+
helpers.isRenderUtil(expressionIdentifier)
67131
);
68132
}) as TSESTree.ExpressionStatement[];
69133
}
@@ -81,19 +145,33 @@ export default createTestingLibraryRule<Options, MessageIds>({
81145
);
82146
}
83147

84-
function reportImplicitReturnSideEffect(node: TSESTree.CallExpression) {
148+
function reportImplicitReturnSideEffect(
149+
node:
150+
| TSESTree.CallExpression
151+
| TSESTree.AssignmentExpression
152+
| TSESTree.SequenceExpression
153+
) {
85154
if (!isCallerWaitFor(node)) {
86155
return;
87156
}
88157

89-
const expressionIdentifier = getPropertyIdentifierNode(node.callee);
90-
if (!expressionIdentifier) {
158+
const expressionIdentifier = isCallExpression(node)
159+
? getPropertyIdentifierNode(node.callee)
160+
: null;
161+
162+
if (
163+
!expressionIdentifier &&
164+
!isRenderInAssignmentExpression(node) &&
165+
!isRenderInSequenceAssignment(node)
166+
) {
91167
return;
92168
}
93169

94170
if (
171+
expressionIdentifier &&
95172
!helpers.isFireEventUtil(expressionIdentifier) &&
96-
!helpers.isUserEventUtil(expressionIdentifier)
173+
!helpers.isUserEventUtil(expressionIdentifier) &&
174+
!helpers.isRenderUtil(expressionIdentifier)
97175
) {
98176
return;
99177
}
@@ -107,6 +185,8 @@ export default createTestingLibraryRule<Options, MessageIds>({
107185
return {
108186
'CallExpression > ArrowFunctionExpression > BlockStatement': reportSideEffects,
109187
'CallExpression > ArrowFunctionExpression > CallExpression': reportImplicitReturnSideEffect,
188+
'CallExpression > ArrowFunctionExpression > AssignmentExpression': reportImplicitReturnSideEffect,
189+
'CallExpression > ArrowFunctionExpression > SequenceExpression': reportImplicitReturnSideEffect,
110190
'CallExpression > FunctionExpression > BlockStatement': reportSideEffects,
111191
};
112192
},

0 commit comments

Comments
 (0)