Skip to content

Commit aa09b3a

Browse files
committed
feat(no-unnecessary-act): detect error from returns
1 parent b1be7f4 commit aa09b3a

File tree

4 files changed

+202
-18
lines changed

4 files changed

+202
-18
lines changed

lib/node-utils/index.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
isLiteral,
1818
isMemberExpression,
1919
isReturnStatement,
20+
isVariableDeclaration,
2021
} from './is-node-of-type';
2122

2223
export * from './is-node-of-type';
@@ -510,3 +511,27 @@ export function hasImportMatch(
510511

511512
return importNode.local.name === identifierName;
512513
}
514+
515+
export function getStatementCallExpression(
516+
statement: TSESTree.Statement
517+
): TSESTree.CallExpression | undefined {
518+
if (
519+
isExpressionStatement(statement) &&
520+
isCallExpression(statement.expression)
521+
) {
522+
return statement.expression;
523+
}
524+
525+
if (isReturnStatement(statement) && isCallExpression(statement.argument)) {
526+
return statement.argument;
527+
}
528+
529+
if (isVariableDeclaration(statement)) {
530+
for (const declaration of statement.declarations) {
531+
if (isCallExpression(declaration.init)) {
532+
return declaration.init;
533+
}
534+
}
535+
}
536+
return undefined;
537+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,6 @@ export const isObjectExpression = isNodeOfType(AST_NODE_TYPES.ObjectExpression);
3434
export const isObjectPattern = isNodeOfType(AST_NODE_TYPES.ObjectPattern);
3535
export const isProperty = isNodeOfType(AST_NODE_TYPES.Property);
3636
export const isReturnStatement = isNodeOfType(AST_NODE_TYPES.ReturnStatement);
37+
export const isVariableDeclaration = isNodeOfType(
38+
AST_NODE_TYPES.VariableDeclaration
39+
);

lib/rules/no-unnecessary-act.ts

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import { createTestingLibraryRule } from '../create-testing-library-rule';
33
import {
44
getDeepestIdentifierNode,
55
getPropertyIdentifierNode,
6-
isCallExpression,
7-
isExpressionStatement,
6+
getStatementCallExpression,
87
} from '../node-utils';
98

109
export const RULE_NAME = 'no-unnecessary-act';
@@ -32,20 +31,21 @@ export default createTestingLibraryRule<[], MessageIds>({
3231
defaultOptions: [],
3332

3433
create(context, _, helpers) {
34+
/**
35+
* Determines whether a given list of statements has some call non-related to Testing Library utils.
36+
*/
3537
function hasNonTestingLibraryCall(
3638
statements: TSESTree.Statement[]
3739
): boolean {
3840
// TODO: refactor to use Array.every
3941
for (const statement of statements) {
40-
if (!isExpressionStatement(statement)) {
41-
continue;
42-
}
42+
const callExpression = getStatementCallExpression(statement);
4343

44-
if (!isCallExpression(statement.expression)) {
44+
if (!callExpression) {
4545
continue;
4646
}
4747

48-
const identifier = getDeepestIdentifierNode(statement.expression);
48+
const identifier = getDeepestIdentifierNode(callExpression);
4949

5050
if (!identifier) {
5151
continue;
@@ -61,7 +61,7 @@ export default createTestingLibraryRule<[], MessageIds>({
6161
return false;
6262
}
6363

64-
function checkNoUnnecessaryAct(
64+
function checkNoUnnecessaryActFromBlockStatement(
6565
blockStatementNode: TSESTree.BlockStatement
6666
) {
6767
const callExpressionNode = blockStatementNode?.parent?.parent as
@@ -96,10 +96,49 @@ export default createTestingLibraryRule<[], MessageIds>({
9696
});
9797
}
9898

99+
function checkNoUnnecessaryActFromImplicitReturn(
100+
node: TSESTree.CallExpression
101+
) {
102+
const nodeIdentifier = getDeepestIdentifierNode(node);
103+
104+
if (!nodeIdentifier) {
105+
return;
106+
}
107+
108+
const parentCallExpression = node?.parent?.parent as
109+
| TSESTree.CallExpression
110+
| undefined;
111+
112+
if (!parentCallExpression) {
113+
return;
114+
}
115+
116+
const parentCallExpressionIdentifier = getPropertyIdentifierNode(
117+
parentCallExpression
118+
);
119+
120+
if (!parentCallExpressionIdentifier) {
121+
return;
122+
}
123+
124+
if (!helpers.isActUtil(parentCallExpressionIdentifier)) {
125+
return;
126+
}
127+
128+
if (!helpers.isTestingLibraryUtil(nodeIdentifier)) {
129+
return;
130+
}
131+
132+
context.report({
133+
node: parentCallExpressionIdentifier,
134+
messageId: 'noUnnecessaryActTestingLibraryUtil',
135+
});
136+
}
137+
99138
return {
100-
'CallExpression > ArrowFunctionExpression > BlockStatement': checkNoUnnecessaryAct,
101-
'CallExpression > FunctionExpression > BlockStatement': checkNoUnnecessaryAct,
102-
// TODO: add selector for call expression > arrow function > implicit return
139+
'CallExpression > ArrowFunctionExpression > BlockStatement': checkNoUnnecessaryActFromBlockStatement,
140+
'CallExpression > FunctionExpression > BlockStatement': checkNoUnnecessaryActFromBlockStatement,
141+
'CallExpression > ArrowFunctionExpression > CallExpression': checkNoUnnecessaryActFromImplicitReturn,
103142
};
104143
},
105144
});

tests/lib/rules/no-unnecessary-act.test.ts

Lines changed: 124 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,12 @@ ruleTester.run(RULE_NAME, rule, {
2525
2626
act(function() {
2727
stuffThatDoesNotUseRTL();
28+
const a = foo();
2829
});
2930
30-
/* TODO get this one back
3131
act(function() {
3232
return stuffThatDoesNotUseRTL();
3333
});
34-
*/
3534
3635
act(() => stuffThatDoesNotUseRTL());
3736
});
@@ -54,11 +53,9 @@ ruleTester.run(RULE_NAME, rule, {
5453
stuffThatDoesNotUseRTL();
5554
});
5655
57-
/* TODO get this one back
5856
act(function() {
5957
return stuffThatDoesNotUseRTL();
6058
});
61-
*/
6259
6360
act(() => stuffThatDoesNotUseRTL());
6461
});
@@ -85,11 +82,9 @@ ruleTester.run(RULE_NAME, rule, {
8582
waitFor();
8683
});
8784
88-
/* TODO get this one back
8985
act(function() {
9086
return waitFor();
9187
});
92-
*/
9388
9489
act(() => waitFor());
9590
});
@@ -200,7 +195,129 @@ ruleTester.run(RULE_NAME, rule, {
200195
],
201196
},
202197

203-
// TODO case: RTL act wrapping RTL calls - callbacks with return
198+
{
199+
code: `// case: RTL act wrapping RTL calls - callbacks with return
200+
import { act, fireEvent, screen, render, waitFor } from '@testing-library/react'
201+
import userEvent from '@testing-library/user-event'
202+
203+
test('invalid case', async () => {
204+
act(() => fireEvent.click(el))
205+
act(() => screen.getByText('blah'))
206+
act(() => findByRole('button'))
207+
act(() => userEvent.click(el))
208+
await act(async () => userEvent.type('hi', el))
209+
act(() => render(foo))
210+
await act(async () => render(fo))
211+
act(() => waitFor(() => {}))
212+
await act(async () => waitFor(() => {}))
213+
214+
act(function () {
215+
return fireEvent.click(el);
216+
});
217+
act(function () {
218+
return screen.getByText('blah');
219+
});
220+
act(function () {
221+
return findByRole('button');
222+
});
223+
act(function () {
224+
return userEvent.click(el);
225+
});
226+
await act(async function () {
227+
return userEvent.type('hi', el);
228+
});
229+
act(function () {
230+
return render(foo);
231+
});
232+
await act(async function () {
233+
return render(fo);
234+
});
235+
act(function () {
236+
return waitFor(() => {});
237+
});
238+
await act(async function () {
239+
return waitFor(() => {});
240+
});
241+
});
242+
`,
243+
errors: [
244+
{ messageId: 'noUnnecessaryActTestingLibraryUtil', line: 6, column: 9 },
245+
{ messageId: 'noUnnecessaryActTestingLibraryUtil', line: 7, column: 9 },
246+
{ messageId: 'noUnnecessaryActTestingLibraryUtil', line: 8, column: 9 },
247+
{ messageId: 'noUnnecessaryActTestingLibraryUtil', line: 9, column: 9 },
248+
{
249+
messageId: 'noUnnecessaryActTestingLibraryUtil',
250+
line: 10,
251+
column: 15,
252+
},
253+
{
254+
messageId: 'noUnnecessaryActTestingLibraryUtil',
255+
line: 11,
256+
column: 9,
257+
},
258+
{
259+
messageId: 'noUnnecessaryActTestingLibraryUtil',
260+
line: 12,
261+
column: 15,
262+
},
263+
{
264+
messageId: 'noUnnecessaryActTestingLibraryUtil',
265+
line: 13,
266+
column: 9,
267+
},
268+
{
269+
messageId: 'noUnnecessaryActTestingLibraryUtil',
270+
line: 14,
271+
column: 15,
272+
},
273+
{
274+
messageId: 'noUnnecessaryActTestingLibraryUtil',
275+
line: 16,
276+
column: 9,
277+
},
278+
{
279+
messageId: 'noUnnecessaryActTestingLibraryUtil',
280+
line: 19,
281+
column: 9,
282+
},
283+
{
284+
messageId: 'noUnnecessaryActTestingLibraryUtil',
285+
line: 22,
286+
column: 9,
287+
},
288+
{
289+
messageId: 'noUnnecessaryActTestingLibraryUtil',
290+
line: 25,
291+
column: 9,
292+
},
293+
{
294+
messageId: 'noUnnecessaryActTestingLibraryUtil',
295+
line: 28,
296+
column: 15,
297+
},
298+
{
299+
messageId: 'noUnnecessaryActTestingLibraryUtil',
300+
line: 31,
301+
column: 9,
302+
},
303+
{
304+
messageId: 'noUnnecessaryActTestingLibraryUtil',
305+
line: 34,
306+
column: 15,
307+
},
308+
{
309+
messageId: 'noUnnecessaryActTestingLibraryUtil',
310+
line: 37,
311+
column: 9,
312+
},
313+
{
314+
messageId: 'noUnnecessaryActTestingLibraryUtil',
315+
line: 40,
316+
column: 15,
317+
},
318+
],
319+
},
320+
204321
// TODO case: RTU act wrapping RTL calls - callbacks with body (BlockStatement)
205322
// TODO case: RTU act wrapping RTL calls - callbacks with return
206323
// TODO case: RTL act wrapping empty callback

0 commit comments

Comments
 (0)