diff --git a/docs/rules/no-render-in-setup.md b/docs/rules/no-render-in-setup.md index d3ef5805..0cdf4cc7 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` (or a custom render function) 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 testing framework setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions. Examples of **incorrect** code for this rule: @@ -20,6 +20,22 @@ it('Should have bar', () => { }); ``` +```js +const setup = () => render(); + +beforeEach(() => { + setup(); +}); + +it('Should have foo', () => { + expect(screen.getByText('foo')).toBeInTheDocument(); +}); + +it('Should have bar', () => { + expect(screen.getByText('bar')).toBeInTheDocument(); +}); +``` + ```js beforeAll(() => { render(); @@ -44,10 +60,18 @@ it('Should have foo and bar', () => { }); ``` -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. +```js +const setup = () => render(); -``` - "testing-library/no-render-in-setup": ["error", {"renderFunctions": ["renderWithRedux", "renderWithRouter"]}], +beforeEach(() => { + // other stuff... +}); + +it('Should have foo and bar', () => { + setup(); + expect(screen.getByText('foo')).toBeInTheDocument(); + expect(screen.getByText('bar')).toBeInTheDocument(); +}); ``` 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"`. diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 4cda2a2e..dc0dde00 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -321,7 +321,7 @@ interface InnermostFunctionScope extends TSESLintScope.FunctionScope { } export function getInnermostFunctionScope( - context: RuleContext, + context: RuleContext, asyncQueryNode: TSESTree.Identifier ): InnermostFunctionScope | null { const innermostScope = ASTUtils.getInnermostScope( @@ -459,24 +459,6 @@ export function getFunctionName( ); } -// TODO: should be removed after v4 is finished -export function isRenderFunction( - callNode: TSESTree.CallExpression, - renderFunctions: string[] -): boolean { - // returns true for `render` and e.g. `customRenderFn` - // as well as `someLib.render` and `someUtils.customRenderFn` - return renderFunctions.some((name) => { - return ( - (ASTUtils.isIdentifier(callNode.callee) && - name === callNode.callee.name) || - (isMemberExpression(callNode.callee) && - ASTUtils.isIdentifier(callNode.callee.property) && - name === callNode.callee.property.name) - ); - }); -} - // TODO: extract into types file? export type ImportModuleNode = | TSESTree.ImportDeclaration @@ -568,7 +550,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { * Gets the Function node which returns the given Identifier. */ export function getInnermostReturningFunction( - context: RuleContext, + context: RuleContext, node: TSESTree.Identifier ): | TSESTree.FunctionDeclaration diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 3f5bb64e..53107de6 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -27,16 +27,7 @@ export default createTestingLibraryRule({ noDebug: 'Unexpected debug statement', }, fixable: null, - schema: [ - { - type: 'object', - properties: { - renderFunctions: { - type: 'array', - }, - }, - }, - ], + schema: [], }, defaultOptions: [], diff --git a/lib/rules/no-render-in-setup.ts b/lib/rules/no-render-in-setup.ts index 66728e07..6bc5bb0e 100644 --- a/lib/rules/no-render-in-setup.ts +++ b/lib/rules/no-render-in-setup.ts @@ -1,24 +1,18 @@ +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils'; import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils'; -import { - isLiteral, - isProperty, - isObjectPattern, + getDeepestIdentifierNode, + getFunctionName, + getInnermostReturningFunction, isCallExpression, - isRenderFunction, - isImportSpecifier, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-render-in-setup'; export type MessageIds = 'noRenderInSetup'; type Options = [ { allowTestingFrameworkSetupHook?: string; - renderFunctions?: string[]; } ]; @@ -41,127 +35,86 @@ export function findClosestBeforeHook( return findClosestBeforeHook(node.parent, testingFrameworkSetupHooksToFilter); } -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', docs: { - description: 'Disallow the use of `render` in setup functions', + description: + 'Disallow the use of `render` in testing frameworks setup functions', category: 'Best Practices', recommended: false, }, messages: { noRenderInSetup: - 'Move `render` out of `{{name}}` and into individual tests.', + 'Forbidden usage of `render` within testing framework `{{ name }}` setup', }, 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; + create(context, [{ allowTestingFrameworkSetupHook }], helpers) { + const renderWrapperNames: string[] = []; + + function detectRenderWrapper(node: TSESTree.Identifier): void { + const innerFunction = getInnermostReturningFunction(context, node); + + if (innerFunction) { + renderWrapperNames.push(getFunctionName(innerFunction)); + } + } 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) + CallExpression(node) { + const testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter( + (hook) => hook !== allowTestingFrameworkSetupHook ); - if (!testingLibImport) { - return; + const callExpressionIdentifier = getDeepestIdentifierNode(node); + const isRenderIdentifier = helpers.isRenderUtil( + callExpressionIdentifier + ); + + if (isRenderIdentifier) { + detectRenderWrapper(callExpressionIdentifier); } - const declaratorNode = node.parent - .parent as TSESTree.VariableDeclarator; - renderImportedFromTestingLib = - isObjectPattern(declaratorNode.id) && - declaratorNode.id.properties.some( - (property) => - isProperty(property) && - ASTUtils.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 - ); + if ( + !isRenderIdentifier && + !renderWrapperNames.includes(callExpressionIdentifier.name) + ) { + return; } + 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, - }, - }); + if (!beforeHook) { + return; } + + context.report({ + node: callExpressionIdentifier, + messageId: 'noRenderInSetup', + data: { + name: beforeHook.name, + }, + }); }, }; }, diff --git a/lib/rules/render-result-naming-convention.ts b/lib/rules/render-result-naming-convention.ts index 9fb7adbb..b6907864 100644 --- a/lib/rules/render-result-naming-convention.ts +++ b/lib/rules/render-result-naming-convention.ts @@ -46,9 +46,10 @@ export default createTestingLibraryRule({ } return { - 'CallExpression Identifier'(node: TSESTree.Identifier) { - if (helpers.isRenderUtil(node)) { - detectRenderWrapper(node); + CallExpression(node) { + const callExpressionIdentifier = getDeepestIdentifierNode(node); + if (helpers.isRenderUtil(callExpressionIdentifier)) { + detectRenderWrapper(callExpressionIdentifier); } }, VariableDeclarator(node) { diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index 1cf9867a..38a8553b 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -9,28 +9,41 @@ ruleTester.run(RULE_NAME, rule, { { code: ` import { render } from '@testing-library/foo'; + + beforeAll(() => { + doOtherStuff(); + }); + + beforeEach(() => { + doSomethingElse(); + }); + 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 + import { render } from '@testing-library/foo'; + beforeAll(() => { + render(); + }); + `, + options: [{ allowTestingFrameworkSetupHook: 'beforeAll' }], + }, + { + code: ` + import { render } from '@testing-library/foo'; + beforeEach(() => { + render(); + }); + `, + options: [{ allowTestingFrameworkSetupHook: 'beforeEach' }], + }, ...TESTING_FRAMEWORK_SETUP_HOOKS.map((setupHook) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` import { render } from 'imNoTestingLibrary'; ${setupHook}(() => { @@ -43,11 +56,15 @@ ruleTester.run(RULE_NAME, rule, { (setupHook) => setupHook !== allowedSetupHook ); return { + settings: { + 'testing-library/utils-module': 'test-utils', + 'testing-library/custom-renders': ['show', 'renderWithRedux'], + }, code: ` import utils from 'imNoTestingLibrary'; - import { renderWithRedux } from '../test-utils'; + import { show } from '../test-utils'; ${allowedSetupHook}(() => { - renderWithRedux() + show() }) ${disallowedHook}(() => { utils.render() @@ -56,12 +73,12 @@ ruleTester.run(RULE_NAME, rule, { options: [ { allowTestingFrameworkSetupHook: allowedSetupHook, - renderFunctions: ['renderWithRedux'], }, ], }; }), ...TESTING_FRAMEWORK_SETUP_HOOKS.map((setupHook) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` const { render } = require('imNoTestingLibrary') @@ -87,6 +104,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 11, messageId: 'noRenderInSetup', }, ], @@ -100,42 +119,47 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 11, messageId: 'noRenderInSetup', }, ], })), // custom render function ...TESTING_FRAMEWORK_SETUP_HOOKS.map((setupHook) => ({ + settings: { + 'testing-library/utils-module': 'test-utils', + 'testing-library/custom-renders': ['show', 'renderWithRedux'], + }, code: ` - import { renderWithRedux } from '../test-utils'; + import { show } from '../test-utils'; + ${setupHook}(() => { - renderWithRedux() + show() }) `, - options: [ - { - renderFunctions: ['renderWithRedux'], - }, - ], errors: [ { + line: 5, + column: 11, messageId: 'noRenderInSetup', }, ], })), - // call render within a wrapper function ...TESTING_FRAMEWORK_SETUP_HOOKS.map((setupHook) => ({ - code: ` + code: `// call render within a wrapper function import { render } from '@testing-library/foo'; - ${setupHook}(() => { - const wrapper = () => { - render() - } - wrapper() - }) + + const wrapper = () => render() + + ${setupHook}(() => { + wrapper() + }) `, errors: [ { + line: 7, + column: 9, messageId: 'noRenderInSetup', }, ], @@ -158,6 +182,8 @@ ruleTester.run(RULE_NAME, rule, { ], errors: [ { + line: 4, + column: 13, messageId: 'noRenderInSetup', }, ], @@ -172,11 +198,14 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 26, messageId: 'noRenderInSetup', }, ], })), ...TESTING_FRAMEWORK_SETUP_HOOKS.map((setupHook) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` import { render } from 'imNoTestingLibrary'; import * as testUtils from '../test-utils'; @@ -187,13 +216,10 @@ ruleTester.run(RULE_NAME, rule, { render() }) `, - options: [ - { - renderFunctions: ['renderWithRedux'], - }, - ], errors: [ { + line: 5, + column: 21, messageId: 'noRenderInSetup', }, ], @@ -208,6 +234,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 5, + column: 11, messageId: 'noRenderInSetup', }, ],