refactor: address review comments

Author: alessbell

docs/rules/no-render-in-setup.md

@@ -2,7 +2,7 @@
 
 ## Rule Details
 
-This rule disallows the usage of `render` in setup functions (`beforeEach` or `beforeAll`) in favor of a single test with multiple assertions.
+This rule disallows the usage of `render` in setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions.
 
 Examples of **incorrect** code for this rule:
 
@@ -38,5 +38,11 @@ it('Should have foo, bar and baz', () => {
 If you use [custom render functions](https://testing-library.com/docs/example-react-redux) then you can set a config option in your `.eslintrc` to look for these.
 
 ```
-   "testing-library/no-render-in-setup": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}],
+   "testing-library/no-render-in-setup": ["error", {"renderFunctions": ["renderWithRedux", "renderWithRouter"]}],
+```
+
+If you would would like to allow the use of `render` (or custom render function) in _either_ `beforeAll` or `beforeEach`, this can be configured using the option `allowTestingFrameworkSetupHook`. This may be useful if you have configured your tests to [skip auto cleanup](https://testing-library.com/docs/react-testing-library/setup#skipping-auto-cleanup). `allowTestingFrameworkSetupHook` is an enum that accepts either `"beforeAll"` or `"beforeEach"`.
+
+```
+   "testing-library/no-render-in-setup": ["error", {"allowTestingFrameworkSetupHook": "beforeAll"}],
 ```

lib/rules/no-render-in-setup.ts

@@ -1,5 +1,5 @@
 import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
-import { getDocsUrl, BEFORE_HOOKS } from '../utils';
+import { getDocsUrl, TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils';
 import {
   isIdentifier,
   isCallExpression,
@@ -10,18 +10,19 @@ export const RULE_NAME = 'no-render-in-setup';
 export type MessageIds = 'noRenderInSetup';
 
 export function findClosestBeforeHook(
-  node: TSESTree.Node
+  node: TSESTree.Node,
+  testingFrameworkSetupHooksToFilter: string[]
 ): TSESTree.Identifier | null {
   if (node === null) return null;
   if (
     isCallExpression(node) &&
     isIdentifier(node.callee) &&
-    BEFORE_HOOKS.includes(node.callee.name)
+    testingFrameworkSetupHooksToFilter.includes(node.callee.name)
   ) {
     return node.callee;
   }
 
-  return findClosestBeforeHook(node.parent);
+  return findClosestBeforeHook(node.parent, testingFrameworkSetupHooksToFilter);
 }
 
 export default ESLintUtils.RuleCreator(getDocsUrl)({
@@ -35,7 +36,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({
     },
     messages: {
       noRenderInSetup:
-        'Combine assertions into a single test instead of re-rendering the component.',
+        'Move `render` out of `{{name}}` and into individual tests.',
     },
     fixable: null,
     schema: [
@@ -45,20 +46,41 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({
           renderFunctions: {
             type: 'array',
           },
+          allowTestingFrameworkSetupHook: {
+            enum: TESTING_FRAMEWORK_SETUP_HOOKS,
+          },
         },
+        anyOf: [
+          {
+            required: ['renderFunctions'],
+          },
+          {
+            required: ['allowTestingFrameworkSetupHook'],
+          },
+        ],
       },
     ],
   },
   defaultOptions: [
     {
       renderFunctions: [],
+      allowTestingFrameworkSetupHook: '',
     },
   ],
 
-  create(context, [{ renderFunctions }]) {
+  create(context, [{ renderFunctions, allowTestingFrameworkSetupHook }]) {
     return {
       CallExpression(node) {
-        const beforeHook = findClosestBeforeHook(node);
+        let testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS;
+        if (allowTestingFrameworkSetupHook.length !== 0) {
+          testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter(
+            hook => hook !== allowTestingFrameworkSetupHook
+          );
+        }
+        const beforeHook = findClosestBeforeHook(
+          node,
+          testingFrameworkSetupHooksToFilter
+        );
         if (isRenderFunction(node, renderFunctions) && beforeHook) {
           context.report({
             node,

lib/utils.ts

@@ -63,7 +63,7 @@ const ASYNC_UTILS = [
   'waitForDomChange',
 ];
 
-const BEFORE_HOOKS = ['beforeEach', 'beforeAll'];
+const TESTING_FRAMEWORK_SETUP_HOOKS = ['beforeEach', 'beforeAll'];
 
 export {
   getDocsUrl,
@@ -75,6 +75,6 @@ export {
   ASYNC_QUERIES_COMBINATIONS,
   ALL_QUERIES_COMBINATIONS,
   ASYNC_UTILS,
-  BEFORE_HOOKS,
+  TESTING_FRAMEWORK_SETUP_HOOKS,
   LIBRARY_MODULES,
 };

tests/lib/rules/no-render-in-setup.test.ts

@@ -1,5 +1,5 @@
 import { createRuleTester } from '../test-utils';
-import { BEFORE_HOOKS } from '../../../lib/utils';
+import { TESTING_FRAMEWORK_SETUP_HOOKS } from '../../../lib/utils';
 import rule, { RULE_NAME } from '../../../lib/rules/no-render-in-setup';
 
 const ruleTester = createRuleTester({
@@ -17,12 +17,28 @@ ruleTester.run(RULE_NAME, rule, {
         })
       `,
     },
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({
+      code: `
+        ${setupHook}(() => {
+          const wrapper = () => {
+            renderWithRedux(<Component/>)
+          }
+          wrapper();
+        })
+      `,
+      options: [
+        {
+          allowTestingFrameworkSetupHook: setupHook,
+          renderFunctions: ['renderWithRedux'],
+        },
+      ],
+    })),
   ],
 
   invalid: [
-    ...BEFORE_HOOKS.map(beforeHook => ({
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({
       code: `
-        ${beforeHook}(() => {
+        ${setupHook}(() => {
           render(<Component/>)
         })
       `,
@@ -32,9 +48,9 @@ ruleTester.run(RULE_NAME, rule, {
         },
       ],
     })),
-    ...BEFORE_HOOKS.map(beforeHook => ({
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({
       code: `
-        ${beforeHook}(function() {
+        ${setupHook}(function() {
           render(<Component/>)
         })
       `,
@@ -45,9 +61,9 @@ ruleTester.run(RULE_NAME, rule, {
       ],
     })),
     // custom render function
-    ...BEFORE_HOOKS.map(beforeHook => ({
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({
       code: `
-        ${beforeHook}(() => {
+        ${setupHook}(() => {
           renderWithRedux(<Component/>)
         })
       `,
@@ -63,9 +79,9 @@ ruleTester.run(RULE_NAME, rule, {
       ],
     })),
     // call render within a wrapper function
-    ...BEFORE_HOOKS.map(beforeHook => ({
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({
       code: `
-        ${beforeHook}(() => {
+        ${setupHook}(() => {
           const wrapper = () => {
             render(<Component/>)
           }
@@ -78,5 +94,30 @@ ruleTester.run(RULE_NAME, rule, {
         },
       ],
     })),
+    ...TESTING_FRAMEWORK_SETUP_HOOKS.map(allowedSetupHook => {
+      const [disallowedHook] = TESTING_FRAMEWORK_SETUP_HOOKS.filter(
+        setupHook => setupHook !== allowedSetupHook
+      );
+      return {
+        code: `
+        ${disallowedHook}(() => {
+          const wrapper = () => {
+            render(<Component/>)
+          }
+          wrapper();
+        })
+      `,
+        options: [
+          {
+            allowTestingFrameworkSetupHook: allowedSetupHook,
+          },
+        ],
+        errors: [
+          {
+            messageId: 'noRenderInSetup',
+          },
+        ],
+      };
+    }),
   ],
 });