From 3fb536400b62f62810fd28602d4e85a596cb9e9b Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Tue, 4 Aug 2020 15:07:28 -0400 Subject: [PATCH 1/6] feat(no-render-in-setup): adds no-render-in-setup rule, closes #207 --- README.md | 4 ++ docs/rules/no-render-in-setup.md | 42 +++++++++++ lib/index.ts | 2 + lib/node-utils.ts | 9 +++ lib/rules/no-debug.ts | 10 +-- lib/rules/no-manual-cleanup.ts | 3 +- lib/rules/no-render-in-setup.ts | 74 +++++++++++++++++++ lib/utils.ts | 3 + tests/lib/rules/no-render-in-setup.test.ts | 82 ++++++++++++++++++++++ 9 files changed, 218 insertions(+), 11 deletions(-) create mode 100644 docs/rules/no-render-in-setup.md create mode 100644 lib/rules/no-render-in-setup.ts create mode 100644 tests/lib/rules/no-render-in-setup.test.ts diff --git a/README.md b/README.md index 91708dcb..ef1ee654 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-29-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -141,6 +143,7 @@ To enable this configuration use the `extends` property in your | [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | | [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | | +| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | | | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | | [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | @@ -219,6 +222,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d + This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/docs/rules/no-render-in-setup.md b/docs/rules/no-render-in-setup.md new file mode 100644 index 00000000..3612cc90 --- /dev/null +++ b/docs/rules/no-render-in-setup.md @@ -0,0 +1,42 @@ +# Disallow the use of `render` in setup functions (no-render-in-setup) + +## Rule Details + +This rule disallows the usage of `render` in setup functions (`beforeEach` or `beforeAll`) in favor of a single test with multiple assertions. + +Examples of **incorrect** code for this rule: + +```js +beforeEach(() => { + render(); +}); + +it('Should have foo', () => { + expect(screen.getByText('foo')).toBeInTheDocument(); +}); + +it('Should have bar', () => { + expect(screen.getByText('bar')).toBeInTheDocument(); +}); + +it('Should have baz', () => { + expect(screen.getByText('baz')).toBeInTheDocument(); +}); +``` + +Examples of **correct** code for this rule: + +```js +it('Should have foo, bar and baz', () => { + render(); + expect(screen.getByText('foo')).toBeInTheDocument(); + expect(screen.getByText('bar')).toBeInTheDocument(); + expect(screen.getByText('baz')).toBeInTheDocument(); +}); +``` + +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"]}], +``` diff --git a/lib/index.ts b/lib/index.ts index 3cc5ec21..4b457bcf 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -6,6 +6,7 @@ import noAwaitSyncQuery from './rules/no-await-sync-query'; import noDebug from './rules/no-debug'; import noDomImport from './rules/no-dom-import'; import noManualCleanup from './rules/no-manual-cleanup'; +import noRenderInSetup from './rules/no-render-in-setup'; import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback'; import preferExplicitAssert from './rules/prefer-explicit-assert'; import preferPresenceQueries from './rules/prefer-presence-queries'; @@ -22,6 +23,7 @@ const rules = { 'no-debug': noDebug, 'no-dom-import': noDomImport, 'no-manual-cleanup': noManualCleanup, + 'no-render-in-setup': noRenderInSetup, 'no-wait-for-empty-callback': noWaitForEmptyCallback, 'prefer-explicit-assert': preferExplicitAssert, 'prefer-find-by': preferFindBy, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index d25c97a0..e46c9a1c 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -50,6 +50,15 @@ export function isVariableDeclarator( return node && node.type === AST_NODE_TYPES.VariableDeclarator; } +export function isRenderFunction( + callNode: TSESTree.CallExpression, + renderFunctions: string[] +) { + return ['render', ...renderFunctions].some( + name => isIdentifier(callNode.callee) && name === callNode.callee.name + ); +} + export function isObjectPattern( node: TSESTree.Node ): node is TSESTree.ObjectPattern { diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 22bbdc58..418826e1 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -9,19 +9,11 @@ import { isAwaitExpression, isMemberExpression, isImportSpecifier, + isRenderFunction, } from '../node-utils'; export const RULE_NAME = 'no-debug'; -function isRenderFunction( - callNode: TSESTree.CallExpression, - renderFunctions: string[] -) { - return ['render', ...renderFunctions].some( - name => isIdentifier(callNode.callee) && name === callNode.callee.name - ); -} - function isRenderVariableDeclarator( node: TSESTree.VariableDeclarator, renderFunctions: string[] diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index abf7754d..6907f56b 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -21,7 +21,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ meta: { type: 'problem', docs: { - description: ' Disallow the use of `cleanup`', + description: 'Disallow the use of `cleanup`', category: 'Best Practices', recommended: false, }, @@ -121,7 +121,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ messageId: 'noManualCleanup', }); } - } else { defaultRequireFromTestingLibrary = declaratorNode.id; } diff --git a/lib/rules/no-render-in-setup.ts b/lib/rules/no-render-in-setup.ts new file mode 100644 index 00000000..708542b2 --- /dev/null +++ b/lib/rules/no-render-in-setup.ts @@ -0,0 +1,74 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl, BEFORE_HOOKS } from '../utils'; +import { + isIdentifier, + isCallExpression, + isRenderFunction, +} from '../node-utils'; + +export const RULE_NAME = 'no-render-in-setup'; +export type MessageIds = 'noRenderInSetup'; + +export function findClosestBeforeHook( + node: TSESTree.Node +): TSESTree.Identifier | null { + if (node === null) return null; + if ( + isCallExpression(node) && + isIdentifier(node.callee) && + BEFORE_HOOKS.includes(node.callee.name) + ) { + return node.callee; + } + + return findClosestBeforeHook(node.parent); +} + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: 'Disallow the use of `render` in setup functions', + category: 'Best Practices', + recommended: false, + }, + messages: { + noRenderInSetup: + 'Combine assertions into a single test instead of re-rendering the component.', + }, + fixable: null, + schema: [ + { + type: 'object', + properties: { + renderFunctions: { + type: 'array', + }, + }, + }, + ], + }, + defaultOptions: [ + { + renderFunctions: [], + }, + ], + + create(context, [{ renderFunctions }]) { + return { + CallExpression(node) { + const beforeHook = findClosestBeforeHook(node); + if (isRenderFunction(node, renderFunctions) && beforeHook) { + context.report({ + node, + messageId: 'noRenderInSetup', + data: { + name: beforeHook.name, + }, + }); + } + }, + }; + }, +}); diff --git a/lib/utils.ts b/lib/utils.ts index 2652aa08..92891831 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -63,6 +63,8 @@ const ASYNC_UTILS = [ 'waitForDomChange', ]; +const BEFORE_HOOKS = ['beforeEach', 'beforeAll']; + export { getDocsUrl, SYNC_QUERIES_VARIANTS, @@ -73,5 +75,6 @@ export { ASYNC_QUERIES_COMBINATIONS, ALL_QUERIES_COMBINATIONS, ASYNC_UTILS, + BEFORE_HOOKS, LIBRARY_MODULES, }; diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts new file mode 100644 index 00000000..6ced8f30 --- /dev/null +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -0,0 +1,82 @@ +import { createRuleTester } from '../test-utils'; +import { BEFORE_HOOKS } from '../../../lib/utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-render-in-setup'; + +const ruleTester = createRuleTester({ + ecmaFeatures: { + jsx: true, + }, +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + it('Test', () => { + render() + }) + `, + }, + ], + + invalid: [ + ...BEFORE_HOOKS.map(beforeHook => ({ + code: ` + ${beforeHook}(() => { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ...BEFORE_HOOKS.map(beforeHook => ({ + code: ` + ${beforeHook}(function() { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + // custom render function + ...BEFORE_HOOKS.map(beforeHook => ({ + code: ` + ${beforeHook}(() => { + renderWithRedux() + }) + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + // call render within a wrapper function + ...BEFORE_HOOKS.map(beforeHook => ({ + code: ` + ${beforeHook}(() => { + const wrapper = () => { + render() + } + wrapper(); + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ], +}); From 966f5213c9f4797f4fa6a5da4e8c384e7d59304c Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Wed, 5 Aug 2020 18:49:04 -0400 Subject: [PATCH 2/6] refactor: address review comments --- docs/rules/no-render-in-setup.md | 10 +++- lib/rules/no-render-in-setup.ts | 36 ++++++++++--- lib/utils.ts | 4 +- tests/lib/rules/no-render-in-setup.test.ts | 59 ++++++++++++++++++---- 4 files changed, 89 insertions(+), 20 deletions(-) diff --git a/docs/rules/no-render-in-setup.md b/docs/rules/no-render-in-setup.md index 3612cc90..bfd94a63 100644 --- a/docs/rules/no-render-in-setup.md +++ b/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"}], ``` diff --git a/lib/rules/no-render-in-setup.ts b/lib/rules/no-render-in-setup.ts index 708542b2..48022243 100644 --- a/lib/rules/no-render-in-setup.ts +++ b/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, diff --git a/lib/utils.ts b/lib/utils.ts index 92891831..18d3d7f9 100644 --- a/lib/utils.ts +++ b/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, }; diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index 6ced8f30..80d998d9 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/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() + } + wrapper(); + }) + `, + options: [ + { + allowTestingFrameworkSetupHook: setupHook, + renderFunctions: ['renderWithRedux'], + }, + ], + })), ], invalid: [ - ...BEFORE_HOOKS.map(beforeHook => ({ + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` - ${beforeHook}(() => { + ${setupHook}(() => { render() }) `, @@ -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() }) `, @@ -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() }) `, @@ -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() } @@ -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() + } + wrapper(); + }) + `, + options: [ + { + allowTestingFrameworkSetupHook: allowedSetupHook, + }, + ], + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + }; + }), ], }); From ef1d4d951a2aaf1c2bfe219894191a2142f1f45e Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Thu, 6 Aug 2020 09:51:52 -0400 Subject: [PATCH 3/6] Update docs/rules/no-render-in-setup.md Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> --- docs/rules/no-render-in-setup.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/no-render-in-setup.md b/docs/rules/no-render-in-setup.md index bfd94a63..c95552cd 100644 --- a/docs/rules/no-render-in-setup.md +++ b/docs/rules/no-render-in-setup.md @@ -41,7 +41,7 @@ If you use [custom render functions](https://testing-library.com/docs/example-re "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"`. +If you 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"}], From 2e469ba041a0d311345d3e1ea3fb1a298be0f505 Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Thu, 6 Aug 2020 16:53:48 -0400 Subject: [PATCH 4/6] fix: updates docs, adds tests --- docs/rules/no-render-in-setup.md | 21 +++++-- lib/node-utils.ts | 13 ++++- tests/lib/rules/no-render-in-setup.test.ts | 68 +++++++++++++++++++--- 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/docs/rules/no-render-in-setup.md b/docs/rules/no-render-in-setup.md index c95552cd..d3ef5805 100644 --- a/docs/rules/no-render-in-setup.md +++ b/docs/rules/no-render-in-setup.md @@ -2,7 +2,7 @@ ## Rule Details -This rule disallows the usage of `render` in setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions. +This rule disallows the usage of `render` (or a custom render function) in setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions. Examples of **incorrect** code for this rule: @@ -18,20 +18,29 @@ it('Should have foo', () => { it('Should have bar', () => { expect(screen.getByText('bar')).toBeInTheDocument(); }); +``` + +```js +beforeAll(() => { + render(); +}); -it('Should have baz', () => { - expect(screen.getByText('baz')).toBeInTheDocument(); +it('Should have foo', () => { + expect(screen.getByText('foo')).toBeInTheDocument(); +}); + +it('Should have bar', () => { + expect(screen.getByText('bar')).toBeInTheDocument(); }); ``` Examples of **correct** code for this rule: ```js -it('Should have foo, bar and baz', () => { +it('Should have foo and bar', () => { render(); expect(screen.getByText('foo')).toBeInTheDocument(); expect(screen.getByText('bar')).toBeInTheDocument(); - expect(screen.getByText('baz')).toBeInTheDocument(); }); ``` @@ -41,7 +50,7 @@ If you use [custom render functions](https://testing-library.com/docs/example-re "testing-library/no-render-in-setup": ["error", {"renderFunctions": ["renderWithRedux", "renderWithRouter"]}], ``` -If you 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"`. +If you would like to allow the use of `render` (or a 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"}], diff --git a/lib/node-utils.ts b/lib/node-utils.ts index e46c9a1c..06e2468d 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -54,9 +54,16 @@ export function isRenderFunction( callNode: TSESTree.CallExpression, renderFunctions: string[] ) { - return ['render', ...renderFunctions].some( - name => isIdentifier(callNode.callee) && name === callNode.callee.name - ); + // returns true for `render` and e.g. `customRenderFn` + // as well as `someLib.render` and `someUtils.customRenderFn` + return ['render', ...renderFunctions].some(name => { + return ( + (isIdentifier(callNode.callee) && name === callNode.callee.name) || + (isMemberExpression(callNode.callee) && + isIdentifier(callNode.callee.property) && + name === callNode.callee.property.name) + ); + }); } export function isObjectPattern( diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index 80d998d9..41d00443 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -17,13 +17,11 @@ ruleTester.run(RULE_NAME, rule, { }) `, }, + // test config options ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` ${setupHook}(() => { - const wrapper = () => { - renderWithRedux() - } - wrapper(); + renderWithRedux() }) `, options: [ @@ -33,6 +31,14 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + // // test usage of a non-Testing Library render fn + // ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + // code: `import { render } from 'imNoTestingLibrary'; + + // ${setupHook}(() => { + // render() + // })`, + // })), ], invalid: [ @@ -85,7 +91,7 @@ ruleTester.run(RULE_NAME, rule, { const wrapper = () => { render() } - wrapper(); + wrapper() }) `, errors: [ @@ -101,10 +107,7 @@ ruleTester.run(RULE_NAME, rule, { return { code: ` ${disallowedHook}(() => { - const wrapper = () => { - render() - } - wrapper(); + render() }) `, options: [ @@ -119,5 +122,52 @@ ruleTester.run(RULE_NAME, rule, { ], }; }), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: `import { render } from '@testing-library/foo'; + + ${setupHook}(() => { + render() + })`, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: `import * as testingLibrary from '@testing-library/foo'; + + ${setupHook}(() => { + testingLibrary.render() + })`, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: `import { render } from 'imNoTestingLibrary'; + import * as testUtils from '../utils'; + + ${setupHook}(() => { + testUtils.renderWithRedux() + }) + + it('Test', () => { + render() + }) + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), ], }); From 08131945eba4eee7a26b31192dd6ae310cc02eb5 Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Fri, 7 Aug 2020 11:06:54 -0400 Subject: [PATCH 5/6] fix: handle require, add tests --- lib/node-utils.ts | 2 +- lib/rules/no-debug.ts | 10 +- lib/rules/no-render-in-setup.ts | 60 +++++++++++- tests/lib/rules/no-render-in-setup.test.ts | 105 ++++++++++++++------- 4 files changed, 134 insertions(+), 43 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 06e2468d..c5099a6f 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -56,7 +56,7 @@ export function isRenderFunction( ) { // returns true for `render` and e.g. `customRenderFn` // as well as `someLib.render` and `someUtils.customRenderFn` - return ['render', ...renderFunctions].some(name => { + return renderFunctions.some(name => { return ( (isIdentifier(callNode.callee) && name === callNode.callee.name) || (isMemberExpression(callNode.callee) && diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 418826e1..f848280d 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -22,15 +22,15 @@ function isRenderVariableDeclarator( if (isAwaitExpression(node.init)) { return ( node.init.argument && - isRenderFunction( - node.init.argument as TSESTree.CallExpression, - renderFunctions - ) + isRenderFunction(node.init.argument as TSESTree.CallExpression, [ + 'render', + ...renderFunctions, + ]) ); } else { return ( isCallExpression(node.init) && - isRenderFunction(node.init, renderFunctions) + isRenderFunction(node.init, ['render', ...renderFunctions]) ); } } diff --git a/lib/rules/no-render-in-setup.ts b/lib/rules/no-render-in-setup.ts index 48022243..c93e9e53 100644 --- a/lib/rules/no-render-in-setup.ts +++ b/lib/rules/no-render-in-setup.ts @@ -1,9 +1,13 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { getDocsUrl, TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils'; import { + isLiteral, + isProperty, isIdentifier, + isObjectPattern, isCallExpression, isRenderFunction, + isImportSpecifier, } from '../node-utils'; export const RULE_NAME = 'no-render-in-setup'; @@ -69,7 +73,53 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ ], create(context, [{ renderFunctions, allowTestingFrameworkSetupHook }]) { + let renderImportedFromTestingLib = false; + let wildcardImportName: string | null = null; + return { + // checks if import has shape: + // import * as dtl from '@testing-library/dom'; + 'ImportDeclaration[source.value=/testing-library/] ImportNamespaceSpecifier'( + node: TSESTree.ImportNamespaceSpecifier + ) { + wildcardImportName = node.local && node.local.name; + }, + // checks if `render` is imported from a '@testing-library/foo' + 'ImportDeclaration[source.value=/testing-library/]'( + node: TSESTree.ImportDeclaration + ) { + renderImportedFromTestingLib = node.specifiers.some(specifier => { + return ( + isImportSpecifier(specifier) && specifier.local.name === 'render' + ); + }); + }, + [`VariableDeclarator > CallExpression > Identifier[name="require"]`]( + node: TSESTree.Identifier + ) { + if (!isCallExpression(node.parent)) return; + const { arguments: callExpressionArgs } = node.parent; + const literalNodeScreenModuleName = callExpressionArgs.find( + args => + isLiteral(args) && + typeof args.value === 'string' && + RegExp(/testing-library/, 'g').test(args.value) + ); + if (!literalNodeScreenModuleName) { + return; + } + const declaratorNode = node.parent + .parent as TSESTree.VariableDeclarator; + + renderImportedFromTestingLib = + isObjectPattern(declaratorNode.id) && + declaratorNode.id.properties.some( + property => + isProperty(property) && + isIdentifier(property.key) && + property.key.name === 'render' + ); + }, CallExpression(node) { let testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS; if (allowTestingFrameworkSetupHook.length !== 0) { @@ -81,7 +131,15 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ node, testingFrameworkSetupHooksToFilter ); - if (isRenderFunction(node, renderFunctions) && beforeHook) { + // if `render` is imported from a @testing-library/foo or + // imported with a wildcard, add `render` to the list of + // disallowed render functions + const disallowedRenderFns = + renderImportedFromTestingLib || wildcardImportName + ? ['render', ...renderFunctions] + : renderFunctions; + + if (isRenderFunction(node, disallowedRenderFns) && beforeHook) { context.report({ node, messageId: 'noRenderInSetup', diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index 41d00443..ea23a036 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -12,6 +12,7 @@ ruleTester.run(RULE_NAME, rule, { valid: [ { code: ` + import { render } from '@testing-library/foo'; it('Test', () => { render() }) @@ -20,6 +21,7 @@ ruleTester.run(RULE_NAME, rule, { // test config options ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` + import { renderWithRedux } from '../test-utils'; ${setupHook}(() => { renderWithRedux() }) @@ -31,19 +33,44 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - // // test usage of a non-Testing Library render fn - // ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ - // code: `import { render } from 'imNoTestingLibrary'; - - // ${setupHook}(() => { - // render() - // })`, - // })), + // test usage of a non-Testing Library render fn + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + import { render } from 'imNoTestingLibrary'; + ${setupHook}(() => { + render() + }) + `, + })), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(allowedSetupHook => { + const [disallowedHook] = TESTING_FRAMEWORK_SETUP_HOOKS.filter( + setupHook => setupHook !== allowedSetupHook + ); + return { + code: ` + import utils from 'imNoTestingLibrary'; + import { renderWithRedux } from '../test-utils'; + ${allowedSetupHook}(() => { + renderWithRedux() + }) + ${disallowedHook}(() => { + utils.render() + }) + `, + options: [ + { + allowTestingFrameworkSetupHook: allowedSetupHook, + renderFunctions: ['renderWithRedux'], + }, + ], + }; + }), ], invalid: [ ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` + import { render } from '@testing-library/foo'; ${setupHook}(() => { render() }) @@ -56,6 +83,7 @@ ruleTester.run(RULE_NAME, rule, { })), ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` + import { render } from '@testing-library/foo'; ${setupHook}(function() { render() }) @@ -69,6 +97,7 @@ ruleTester.run(RULE_NAME, rule, { // custom render function ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` + import { renderWithRedux } from '../test-utils'; ${setupHook}(() => { renderWithRedux() }) @@ -87,6 +116,7 @@ ruleTester.run(RULE_NAME, rule, { // call render within a wrapper function ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ code: ` + import { render } from '@testing-library/foo'; ${setupHook}(() => { const wrapper = () => { render() @@ -106,10 +136,11 @@ ruleTester.run(RULE_NAME, rule, { ); return { code: ` - ${disallowedHook}(() => { - render() - }) - `, + import { render } from '@testing-library/foo'; + ${disallowedHook}(() => { + render() + }) + `, options: [ { allowTestingFrameworkSetupHook: allowedSetupHook, @@ -123,11 +154,12 @@ ruleTester.run(RULE_NAME, rule, { }; }), ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ - code: `import { render } from '@testing-library/foo'; - - ${setupHook}(() => { - render() - })`, + code: ` + import * as testingLibrary from '@testing-library/foo'; + ${setupHook}(() => { + testingLibrary.render() + }) + `, errors: [ { messageId: 'noRenderInSetup', @@ -135,11 +167,21 @@ ruleTester.run(RULE_NAME, rule, { ], })), ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ - code: `import * as testingLibrary from '@testing-library/foo'; - - ${setupHook}(() => { - testingLibrary.render() - })`, + code: ` + import { render } from 'imNoTestingLibrary'; + import * as testUtils from '../test-utils'; + ${setupHook}(() => { + testUtils.renderWithRedux() + }) + it('Test', () => { + render() + }) + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], errors: [ { messageId: 'noRenderInSetup', @@ -147,22 +189,13 @@ ruleTester.run(RULE_NAME, rule, { ], })), ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ - code: `import { render } from 'imNoTestingLibrary'; - import * as testUtils from '../utils'; - - ${setupHook}(() => { - testUtils.renderWithRedux() - }) + code: ` + const { render } = require('@testing-library/foo') - it('Test', () => { - render() - }) + ${setupHook}(() => { + render() + }) `, - options: [ - { - renderFunctions: ['renderWithRedux'], - }, - ], errors: [ { messageId: 'noRenderInSetup', From 542f094e6b02fc9f6256d651cf44ce421b1dba12 Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Fri, 7 Aug 2020 11:18:43 -0400 Subject: [PATCH 6/6] fix: code coverage --- lib/rules/no-render-in-setup.ts | 9 +++++---- tests/lib/rules/no-render-in-setup.test.ts | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-render-in-setup.ts b/lib/rules/no-render-in-setup.ts index c93e9e53..ca666a51 100644 --- a/lib/rules/no-render-in-setup.ts +++ b/lib/rules/no-render-in-setup.ts @@ -97,15 +97,16 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ [`VariableDeclarator > CallExpression > Identifier[name="require"]`]( node: TSESTree.Identifier ) { - if (!isCallExpression(node.parent)) return; - const { arguments: callExpressionArgs } = node.parent; - const literalNodeScreenModuleName = callExpressionArgs.find( + const { + arguments: callExpressionArgs, + } = node.parent as TSESTree.CallExpression; + const testingLibImport = callExpressionArgs.find( args => isLiteral(args) && typeof args.value === 'string' && RegExp(/testing-library/, 'g').test(args.value) ); - if (!literalNodeScreenModuleName) { + if (!testingLibImport) { return; } const declaratorNode = node.parent diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index ea23a036..1f4a529d 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -65,6 +65,20 @@ ruleTester.run(RULE_NAME, rule, { ], }; }), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + const { render } = require('imNoTestingLibrary') + + ${setupHook}(() => { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), ], invalid: [