(feat:no-unnecessary-act): adds no-unnecessary-act rule, closes #259 #292
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,26 @@ | ||||||||
| # Disallow the use of `act` when wrapping Testing Library functions or functions with an empty body | ||||||||
|
|
||||||||
| ## Rule Details | ||||||||
|
|
||||||||
| This rule disallows the usage of `act` when using it to wrap Testing Library functions, or functions with an empty body, which suppresses valid warnings. For more information, see [https://kcd.im/react-act](https://kcd.im/react-act). | ||||||||
|
|
||||||||
| Examples of **incorrect** code for this rule: | ||||||||
|
|
||||||||
| ```js | ||||||||
| import { fireEvent, act } from '@testing-library/react'; | ||||||||
|
There was a problem hiding this comment. Use
Suggested change
There was a problem hiding this comment. I would actually use both examples: with |
||||||||
|
|
||||||||
| it('Should have foo', () => { | ||||||||
| act(() => fireEvent.click(el)); | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| }); | ||||||||
| ``` | ||||||||
|
|
||||||||
| Examples of **correct** code for this rule: | ||||||||
|
|
||||||||
| ```js | ||||||||
| import { act } from '@testing-library/react'; | ||||||||
| it('Should have foo and bar', () => { | ||||||||
| act(() => { | ||||||||
| stuffThatDoesNotUseRTL(); | ||||||||
| }); | ||||||||
| }); | ||||||||
| ``` | ||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,85 @@ | ||||||||||||||||||||||||||||||||||||
| import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils'; | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| isBlockStatement, | ||||||||||||||||||||||||||||||||||||
| isCallExpression, | ||||||||||||||||||||||||||||||||||||
| isMemberExpression | ||||||||||||||||||||||||||||||||||||
| } from '../node-utils'; | ||||||||||||||||||||||||||||||||||||
| import { createTestingLibraryRule } from '../create-testing-library-rule'; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export const RULE_NAME = 'no-unnecessary-act'; | ||||||||||||||||||||||||||||||||||||
| export type MessageIds = 'noUnnecessaryAct'; | ||||||||||||||||||||||||||||||||||||
| type Options = []; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const ACT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(act)$/]'; | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. I realized after migrating most of the rules to v4 that this limits the way we try to find the corresponding utils. This won't report Instead, it's better to just create a listener for You can create this new The Not sure if I'm explaining myself here really well, probably easier if you check the idea in that PR and let me know if you want me to go over any particular point 😅 |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export default createTestingLibraryRule<Options, MessageIds>({ | ||||||||||||||||||||||||||||||||||||
| name: RULE_NAME, | ||||||||||||||||||||||||||||||||||||
| meta: { | ||||||||||||||||||||||||||||||||||||
| type: 'problem', | ||||||||||||||||||||||||||||||||||||
| docs: { | ||||||||||||||||||||||||||||||||||||
| description: | ||||||||||||||||||||||||||||||||||||
| 'Disallow the use of `act` when wrapping Testing Library functions or functions with an empty body', | ||||||||||||||||||||||||||||||||||||
| category: 'Best Practices', | ||||||||||||||||||||||||||||||||||||
| recommended: false | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| messages: { | ||||||||||||||||||||||||||||||||||||
| noUnnecessaryAct: | ||||||||||||||||||||||||||||||||||||
| 'Avoid wrapping Testing Library functions in `act` as they are wrapped in act automatically, and avoid wrapping noop functions in `act` as they suppress valid warnings.' | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| fixable: null, | ||||||||||||||||||||||||||||||||||||
| schema: [] | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| defaultOptions: [], | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| create(context, _, helpers) { | ||||||||||||||||||||||||||||||||||||
| function reportIfUnnecessary( | ||||||||||||||||||||||||||||||||||||
| node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| let isEmpty = false; | ||||||||||||||||||||||||||||||||||||
| let callsTL = false; | ||||||||||||||||||||||||||||||||||||
| const callsOnlyTLFns = (body: Array<TSESTree.Node>): boolean => | ||||||||||||||||||||||||||||||||||||
| body.every((node: TSESTree.ExpressionStatement) => { | ||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||
| isCallExpression(node.expression) && | ||||||||||||||||||||||||||||||||||||
| isMemberExpression(node.expression.callee) && | ||||||||||||||||||||||||||||||||||||
| isCallExpression(node.expression.callee.object) && | ||||||||||||||||||||||||||||||||||||
| helpers.isNodeComingFromTestingLibrary(node.expression.callee) | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+51
There was a problem hiding this comment. really minor suggestion, but you can drop the if clause if you want and just return the expression
Suggested change
|
||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||
| isBlockStatement(node.body) && | ||||||||||||||||||||||||||||||||||||
| node.body.body.length === 0 && | ||||||||||||||||||||||||||||||||||||
| isCallExpression(node.parent) && | ||||||||||||||||||||||||||||||||||||
| ASTUtils.isIdentifier(node.parent.callee) | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| isEmpty = true; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||
| isCallExpression(node.body) && | ||||||||||||||||||||||||||||||||||||
| isMemberExpression(node.body.callee) && | ||||||||||||||||||||||||||||||||||||
| helpers.isNodeComingFromTestingLibrary(node.body.callee) | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| callsTL = true; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||
| isEmpty || | ||||||||||||||||||||||||||||||||||||
| callsTL || | ||||||||||||||||||||||||||||||||||||
| (isBlockStatement(node.body) && callsOnlyTLFns(node.body.body)) | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| context.report({ | ||||||||||||||||||||||||||||||||||||
| node, | ||||||||||||||||||||||||||||||||||||
| loc: node.body.loc.start, | ||||||||||||||||||||||||||||||||||||
| messageId: 'noUnnecessaryAct' | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| [`${ACT_EXPRESSION_QUERY} > ArrowFunctionExpression`]: reportIfUnnecessary, | ||||||||||||||||||||||||||||||||||||
| [`${ACT_EXPRESSION_QUERY} > FunctionExpression`]: reportIfUnnecessary | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.