diff --git a/README.md b/README.md index f204cd0f..a7efc643 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,7 @@ To enable this configuration use the `extends` property in your | [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | | | [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | | [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-container](docs/rules/no-container.md) | Disallow the use of `container` methods | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [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` | | | diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md new file mode 100644 index 00000000..658879e2 --- /dev/null +++ b/docs/rules/no-container.md @@ -0,0 +1,44 @@ +# Disallow the use of `container` methods (no-container) + +By using `container` methods like `.querySelector` you may lose a lot of the confidence that the user can really interact with your UI. Also, the test becomes harder to read, and it will break more frequently. + +This applies to Testing Library frameworks built on top of **DOM Testing Library** + +## Rule Details + +This rule aims to disallow the use of `container` methods in your tests. + +Examples of **incorrect** code for this rule: + +```js +const { container } = render(); +const button = container.querySelector('.btn-primary'); +``` + +```js +const { container: alias } = render(); +const button = alias.querySelector('.btn-primary'); +``` + +```js +const view = render(); +const button = view.container.getElementsByClassName('.btn-primary'); +``` + +Examples of **correct** code for this rule: + +```js +render(); +screen.getByRole('button', { name: /click me/i }); +``` + +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-container": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}], +``` + +## Further Reading + +- [about the `container` element](https://testing-library.com/docs/react-testing-library/api#container-1) +- [querying with `screen`](https://testing-library.com/docs/dom-testing-library/api-queries#screen) diff --git a/lib/index.ts b/lib/index.ts index 886c2cd6..80b806f4 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -3,6 +3,7 @@ import awaitAsyncUtils from './rules/await-async-utils'; import awaitFireEvent from './rules/await-fire-event'; import consistentDataTestid from './rules/consistent-data-testid'; import noAwaitSyncQuery from './rules/no-await-sync-query'; +import noContainer from './rules/no-container'; import noDebug from './rules/no-debug'; import noDomImport from './rules/no-dom-import'; import noManualCleanup from './rules/no-manual-cleanup'; @@ -20,6 +21,7 @@ const rules = { 'await-fire-event': awaitFireEvent, 'consistent-data-testid': consistentDataTestid, 'no-await-sync-query': noAwaitSyncQuery, + 'no-container': noContainer, 'no-debug': noDebug, 'no-dom-import': noDomImport, 'no-manual-cleanup': noManualCleanup, @@ -53,6 +55,7 @@ export = { plugins: ['testing-library'], rules: { ...recommendedRules, + 'testing-library/no-container': 'error', 'testing-library/no-debug': 'warn', 'testing-library/no-dom-import': ['error', 'angular'], }, @@ -61,6 +64,7 @@ export = { plugins: ['testing-library'], rules: { ...recommendedRules, + 'testing-library/no-container': 'error', 'testing-library/no-debug': 'warn', 'testing-library/no-dom-import': ['error', 'react'], }, @@ -70,6 +74,7 @@ export = { rules: { ...recommendedRules, 'testing-library/await-fire-event': 'error', + 'testing-library/no-container': 'error', 'testing-library/no-debug': 'warn', 'testing-library/no-dom-import': ['error', 'vue'], }, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 544df8fc..60e88db3 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -114,3 +114,36 @@ export function isArrowFunctionExpression( ): node is TSESTree.ArrowFunctionExpression { return node && node.type === 'ArrowFunctionExpression'; } + +function isRenderFunction( + callNode: TSESTree.CallExpression, + renderFunctions: string[] +) { + return ['render', ...renderFunctions].some( + name => isIdentifier(callNode.callee) && name === callNode.callee.name + ); +} + +export function isRenderVariableDeclarator( + node: TSESTree.VariableDeclarator, + renderFunctions: string[] +) { + if (node.init) { + if (isAwaitExpression(node.init)) { + return ( + node.init.argument && + isRenderFunction( + node.init.argument as TSESTree.CallExpression, + renderFunctions + ) + ); + } else { + return ( + isCallExpression(node.init) && + isRenderFunction(node.init, renderFunctions) + ); + } + } + + return false; +} diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts new file mode 100644 index 00000000..cd7c67a9 --- /dev/null +++ b/lib/rules/no-container.ts @@ -0,0 +1,120 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl } from '../utils'; +import { + isIdentifier, + isMemberExpression, + isObjectPattern, + isProperty, + isRenderVariableDeclarator, +} from '../node-utils'; + +export const RULE_NAME = 'no-container'; + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: 'Disallow the use of container methods', + category: 'Best Practices', + recommended: 'error', + }, + messages: { + noContainer: + 'Avoid using container methods. Prefer using the methods from Testing Library, such as "getByRole()"', + }, + fixable: null, + schema: [ + { + type: 'object', + properties: { + renderFunctions: { + type: 'array', + }, + }, + }, + ], + }, + defaultOptions: [ + { + renderFunctions: [], + }, + ], + + create(context, [options]) { + const { renderFunctions } = options; + const destructuredContainerPropNames: string[] = []; + let renderWrapperName: string = null; + let containerName: string = null; + let containerCallsMethod = false; + + function showErrorIfChainedContainerMethod( + innerNode: TSESTree.MemberExpression + ) { + if (isMemberExpression(innerNode)) { + if (isIdentifier(innerNode.object)) { + const isContainerName = innerNode.object.name === containerName; + const isRenderWrapper = innerNode.object.name === renderWrapperName; + + containerCallsMethod = + isIdentifier(innerNode.property) && + innerNode.property.name === 'container' && + isRenderWrapper; + + if (isContainerName || containerCallsMethod) { + context.report({ + node: innerNode, + messageId: 'noContainer', + }); + } + } + showErrorIfChainedContainerMethod( + innerNode.object as TSESTree.MemberExpression + ); + } + } + + return { + VariableDeclarator(node) { + if (isRenderVariableDeclarator(node, renderFunctions)) { + if (isObjectPattern(node.id)) { + const containerIndex = node.id.properties.findIndex( + property => + isProperty(property) && + isIdentifier(property.key) && + property.key.name === 'container' + ); + const nodeValue = + containerIndex !== -1 && node.id.properties[containerIndex].value; + if (isIdentifier(nodeValue)) { + containerName = nodeValue.name; + } else { + isObjectPattern(nodeValue) && + nodeValue.properties.forEach( + property => + isProperty(property) && + isIdentifier(property.key) && + destructuredContainerPropNames.push(property.key.name) + ); + } + } else { + renderWrapperName = isIdentifier(node.id) && node.id.name; + } + } + }, + + CallExpression(node: TSESTree.CallExpression) { + if (isMemberExpression(node.callee)) { + showErrorIfChainedContainerMethod(node.callee); + } else { + isIdentifier(node.callee) && + destructuredContainerPropNames.includes(node.callee.name) && + context.report({ + node, + messageId: 'noContainer', + }); + } + }, + }; + }, +}); diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 22bbdc58..752a1c39 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -6,46 +6,13 @@ import { isIdentifier, isCallExpression, isLiteral, - isAwaitExpression, isMemberExpression, isImportSpecifier, + isRenderVariableDeclarator, } 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[] -) { - if (node.init) { - if (isAwaitExpression(node.init)) { - return ( - node.init.argument && - isRenderFunction( - node.init.argument as TSESTree.CallExpression, - renderFunctions - ) - ); - } else { - return ( - isCallExpression(node.init) && - isRenderFunction(node.init, renderFunctions) - ); - } - } - - return false; -} - function hasTestingLibraryImportModule( importDeclarationNode: TSESTree.ImportDeclaration ) { diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index 0113c3ea..3c50ac6a 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -9,6 +9,7 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", + "testing-library/no-container": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ "error", @@ -31,6 +32,7 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", + "testing-library/no-container": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ "error", @@ -71,6 +73,7 @@ Object { "testing-library/await-async-utils": "error", "testing-library/await-fire-event": "error", "testing-library/no-await-sync-query": "error", + "testing-library/no-container": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ "error", diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts new file mode 100644 index 00000000..a6a6116a --- /dev/null +++ b/tests/lib/rules/no-container.test.ts @@ -0,0 +1,129 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-container'; + +const ruleTester = createRuleTester({ + ecmaFeatures: { + jsx: true, + }, +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + render(); + screen.getByRole('button', {name: /click me/i}); + `, + }, + { + code: ` + const { container } = render(); + expect(container.firstChild).toBeDefined(); + `, + }, + { + code: ` + const { container: alias } = render(); + expect(alias.firstChild).toBeDefined(); + `, + }, + { + code: ` + function getExampleDOM() { + const container = document.createElement('div'); + container.innerHTML = \` + + + + \`; + const button = container.querySelector('button'); + + button.addEventListener('click', () => console.log('clicked')); + return container; + } + + const exampleDOM = getExampleDOM(); + screen.getByText(exampleDOM, 'Print Username').click(); + `, + }, + { + code: ` + const { container: { firstChild } } = render(); + expect(firstChild).toBeDefined(); + `, + }, + ], + invalid: [ + { + code: ` + const { container } = render(); + const button = container.querySelector('.btn-primary'); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const { container } = render(); + container.querySelector(); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const { container: alias } = render(); + alias.querySelector(); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const view = render(); + const button = view.container.querySelector('.btn-primary'); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const { container: { querySelector } } = render(); + querySelector('foo'); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const { container } = renderWithRedux(); + container.querySelector(); + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + ], +});