fix(no-wait-for-side-effects): false positive inside .then() (#645)

* fix: no-wait-for-side-effects false positive inside .then() Added tests and a mention of edge case to rule documentation Closes #500 * fix: review feedback Remove awaits as the promise is waited by .then() * fix: review feedback Reuse existing helper function to make solution simpler * fix: review feedback Remove await as the promise is waited by .then() * fix: review feedback Remove await as the promise is waited by .then()
testing-library · Sep 13, 2022 · fc6ccf8 · fc6ccf8
1 parent 2cf3883
commit fc6ccf8
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 0 deletions.
diff --git a/docs/rules/no-wait-for-side-effects.md b/docs/rules/no-wait-for-side-effects.md
@@ -73,6 +73,15 @@ Examples of **correct** code for this rule:
     expect(b).toEqual('b');
   });
 
+  // or
+  userEvent.click(button);
+  waitFor(function() {
+    expect(b).toEqual('b');
+  }).then(() => {
+    // Outside of waitFor, e.g. inside a .then() side effects are allowed
+    fireEvent.click(button);
+  });
+
   // or
   render(<App />)
   await waitFor(() => {

diff --git a/lib/rules/no-wait-for-side-effects.ts b/lib/rules/no-wait-for-side-effects.ts
@@ -8,6 +8,7 @@ import {
 	isAssignmentExpression,
 	isCallExpression,
 	isSequenceExpression,
+	hasThenProperty,
 } from '../node-utils';
 
 export const RULE_NAME = 'no-wait-for-side-effects';
@@ -56,6 +57,22 @@ export default createTestingLibraryRule<Options, MessageIds>({
 			);
 		}
 
+		function isCallerThen(
+			node:
+				| TSESTree.AssignmentExpression
+				| TSESTree.BlockStatement
+				| TSESTree.CallExpression
+				| TSESTree.SequenceExpression
+		): boolean {
+			if (!node.parent) {
+				return false;
+			}
+
+			const callExpressionNode = node.parent.parent as TSESTree.CallExpression;
+
+			return hasThenProperty(callExpressionNode.callee);
+		}
+
 		function isRenderInVariableDeclaration(node: TSESTree.Node) {
 			return (
 				isVariableDeclaration(node) &&
@@ -137,6 +154,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
 				return;
 			}
 
+			if (isCallerThen(node)) {
+				return;
+			}
+
 			getSideEffectNodes(node.body).forEach((sideEffectNode) =>
 				context.report({
 					node: sideEffectNode,

diff --git a/tests/lib/rules/no-wait-for-side-effects.test.ts b/tests/lib/rules/no-wait-for-side-effects.test.ts
@@ -99,6 +99,19 @@ ruleTester.run(RULE_NAME, rule, {
           await waitFor(function() {
             expect(b).toEqual('b')
           })
+        `,
+			},
+			{
+				// Issue #500, https://github.com/testing-library/eslint-plugin-testing-library/issues/500
+				code: `
+          import { waitFor } from '${testingFramework}';
+          userEvent.click(button)
+          waitFor(function() {
+            expect(b).toEqual('b')
+          }).then(() => {
+            // Side effects are allowed inside .then()
+            userEvent.click(button);
+          })
         `,
 			},
 		]),
@@ -722,6 +735,41 @@ ruleTester.run(RULE_NAME, rule, {
       `,
 				errors: [{ line: 4, column: 11, messageId: 'noSideEffectsWaitFor' }],
 			} as const,
+			{
+				// Issue #500, https://github.com/testing-library/eslint-plugin-testing-library/issues/500
+				code: `
+        import { waitFor } from '${testingFramework}';
+        waitFor(function() {
+          userEvent.click(button)
+          expect(b).toEqual('b')
+        }).then(() => {
+          userEvent.click(button) // Side effects are allowed inside .then()
+          expect(b).toEqual('b') 
+        })
+      `,
+				errors: [{ line: 4, column: 11, messageId: 'noSideEffectsWaitFor' }],
+			} as const,
+			{
+				// Issue #500, https://github.com/testing-library/eslint-plugin-testing-library/issues/500
+				code: `
+        import { waitFor } from '${testingFramework}';
+        waitFor(function() {
+          userEvent.click(button)
+          expect(b).toEqual('b')
+        }).then(() => {
+          userEvent.click(button) // Side effects are allowed inside .then()
+          expect(b).toEqual('b')
+          await waitFor(() => {
+            fireEvent.keyDown(input, {key: 'ArrowDown'}) // But not if there is a another waitFor with side effects inside the .then()
+            expect(b).toEqual('b')
+          })
+        })
+      `,
+				errors: [
+					{ line: 4, column: 11, messageId: 'noSideEffectsWaitFor' },
+					{ line: 10, column: 13, messageId: 'noSideEffectsWaitFor' },
+				],
+			} as const,
 		]),
 
 		{