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..d3ef5805 --- /dev/null +++ b/docs/rules/no-render-in-setup.md @@ -0,0 +1,57 @@ +# Disallow the use of `render` in setup functions (no-render-in-setup) + +## Rule Details + +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: + +```js +beforeEach(() => { + render(); +}); + +it('Should have foo', () => { + expect(screen.getByText('foo')).toBeInTheDocument(); +}); + +it('Should have bar', () => { + expect(screen.getByText('bar')).toBeInTheDocument(); +}); +``` + +```js +beforeAll(() => { + render(); +}); + +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 and bar', () => { + render(); + expect(screen.getByText('foo')).toBeInTheDocument(); + expect(screen.getByText('bar')).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"]}], +``` + +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/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..c5099a6f 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -50,6 +50,22 @@ export function isVariableDeclarator( return node && node.type === AST_NODE_TYPES.VariableDeclarator; } +export function isRenderFunction( + callNode: TSESTree.CallExpression, + renderFunctions: string[] +) { + // returns true for `render` and e.g. `customRenderFn` + // as well as `someLib.render` and `someUtils.customRenderFn` + return 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( node: TSESTree.Node ): node is TSESTree.ObjectPattern { diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 22bbdc58..f848280d 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[] @@ -30,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-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..ca666a51 --- /dev/null +++ b/lib/rules/no-render-in-setup.ts @@ -0,0 +1,155 @@ +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'; +export type MessageIds = 'noRenderInSetup'; + +export function findClosestBeforeHook( + node: TSESTree.Node, + testingFrameworkSetupHooksToFilter: string[] +): TSESTree.Identifier | null { + if (node === null) return null; + if ( + isCallExpression(node) && + isIdentifier(node.callee) && + testingFrameworkSetupHooksToFilter.includes(node.callee.name) + ) { + return node.callee; + } + + return findClosestBeforeHook(node.parent, testingFrameworkSetupHooksToFilter); +} + +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: + 'Move `render` out of `{{name}}` and into individual tests.', + }, + fixable: null, + schema: [ + { + type: 'object', + properties: { + renderFunctions: { + type: 'array', + }, + allowTestingFrameworkSetupHook: { + enum: TESTING_FRAMEWORK_SETUP_HOOKS, + }, + }, + anyOf: [ + { + required: ['renderFunctions'], + }, + { + required: ['allowTestingFrameworkSetupHook'], + }, + ], + }, + ], + }, + defaultOptions: [ + { + renderFunctions: [], + allowTestingFrameworkSetupHook: '', + }, + ], + + 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 + ) { + 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 (!testingLibImport) { + 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) { + testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter( + hook => hook !== allowTestingFrameworkSetupHook + ); + } + const beforeHook = findClosestBeforeHook( + node, + testingFrameworkSetupHooksToFilter + ); + // 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', + data: { + name: beforeHook.name, + }, + }); + } + }, + }; + }, +}); diff --git a/lib/utils.ts b/lib/utils.ts index 2652aa08..18d3d7f9 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -63,6 +63,8 @@ const ASYNC_UTILS = [ 'waitForDomChange', ]; +const TESTING_FRAMEWORK_SETUP_HOOKS = ['beforeEach', 'beforeAll']; + export { getDocsUrl, SYNC_QUERIES_VARIANTS, @@ -73,5 +75,6 @@ export { ASYNC_QUERIES_COMBINATIONS, ALL_QUERIES_COMBINATIONS, ASYNC_UTILS, + 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 new file mode 100644 index 00000000..1f4a529d --- /dev/null +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -0,0 +1,220 @@ +import { createRuleTester } from '../test-utils'; +import { TESTING_FRAMEWORK_SETUP_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: ` + import { render } from '@testing-library/foo'; + it('Test', () => { + render() + }) + `, + }, + // test config options + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + import { renderWithRedux } from '../test-utils'; + ${setupHook}(() => { + renderWithRedux() + }) + `, + options: [ + { + allowTestingFrameworkSetupHook: setupHook, + renderFunctions: ['renderWithRedux'], + }, + ], + })), + // 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'], + }, + ], + }; + }), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + const { render } = require('imNoTestingLibrary') + + ${setupHook}(() => { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ], + + invalid: [ + ...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 { render } from '@testing-library/foo'; + ${setupHook}(function() { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + // custom render function + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + import { renderWithRedux } from '../test-utils'; + ${setupHook}(() => { + renderWithRedux() + }) + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + // call render within a wrapper function + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + import { render } from '@testing-library/foo'; + ${setupHook}(() => { + const wrapper = () => { + render() + } + wrapper() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(allowedSetupHook => { + const [disallowedHook] = TESTING_FRAMEWORK_SETUP_HOOKS.filter( + setupHook => setupHook !== allowedSetupHook + ); + return { + code: ` + import { render } from '@testing-library/foo'; + ${disallowedHook}(() => { + render() + }) + `, + options: [ + { + allowTestingFrameworkSetupHook: allowedSetupHook, + }, + ], + 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 '../test-utils'; + ${setupHook}(() => { + testUtils.renderWithRedux() + }) + it('Test', () => { + render() + }) + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ...TESTING_FRAMEWORK_SETUP_HOOKS.map(setupHook => ({ + code: ` + const { render } = require('@testing-library/foo') + + ${setupHook}(() => { + render() + }) + `, + errors: [ + { + messageId: 'noRenderInSetup', + }, + ], + })), + ], +});