From 094f53c1106747a68f5725ea62783d55f606999f Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Thu, 18 Jun 2020 20:51:27 -0300 Subject: [PATCH 01/18] feat(no-container): setup new rule on index --- lib/index.ts | 3 +++ tests/__snapshots__/index.test.ts.snap | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/lib/index.ts b/lib/index.ts index 3cc5ec21..1644715e 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'; @@ -19,6 +20,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, @@ -34,6 +36,7 @@ const recommendedRules = { '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/prefer-find-by': 'error', }; diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index fedc3c05..fc281b72 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", @@ -28,6 +29,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", @@ -47,6 +49,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/prefer-find-by": "error", }, } @@ -62,6 +65,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", From da88a9d3c11b729fa9265debf1c73b1800043ac3 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Thu, 18 Jun 2020 20:53:34 -0300 Subject: [PATCH 02/18] refactor(no-container): set rules --- lib/rules/no-container.ts | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 lib/rules/no-container.ts diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts new file mode 100644 index 00000000..7788c82c --- /dev/null +++ b/lib/rules/no-container.ts @@ -0,0 +1,78 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl } from '../utils'; +import { + isCallExpression, + isIdentifier, + isMemberExpression, + isObjectPattern, + isProperty, +} from '../node-utils'; + +export const RULE_NAME = 'no-container'; + +function isRender(callNode: TSESTree.CallExpression) { + return isIdentifier(callNode.callee) && callNode.callee.name === 'render'; +} + +function isRenderVariableDeclarator(node: TSESTree.VariableDeclarator) { + if (node.init) { + if (isCallExpression(node.init)) { + return isRender(node.init); + } + } +} + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: 'Disallow the usage of container methods', + category: 'Best Practices', + recommended: 'error', + }, + messages: { + noContainer: 'Unexpected use of container methods. Prefer the use of "screen.someMethod()".', + }, + fixable: null, + schema: [], + }, + defaultOptions: [], + + create(context) { + let destructuredContainerName = ''; + + return { + VariableDeclarator(node) { + if (isRenderVariableDeclarator(node)) { + if (isObjectPattern(node.id)) { + const containerIndex = node.id.properties.findIndex( + property => + isProperty(property) && + isIdentifier(property.key) && + property.key.name === 'container' + ); + const nodeValue = node.id.properties[containerIndex].value; + destructuredContainerName = + containerIndex !== -1 && + isIdentifier(nodeValue) && + nodeValue.name; + } + } + }, + + [`CallExpression`](node: TSESTree.CallExpression) { + if ( + isMemberExpression(node.callee) && + isIdentifier(node.callee.object) && + node.callee.object.name === destructuredContainerName + ) { + context.report({ + node, + messageId: 'noContainer', + }); + } + }, + }; + }, +}); From ee369e5e1ddf93333c067b0a5b745eb9df84ce01 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Thu, 18 Jun 2020 20:54:25 -0300 Subject: [PATCH 03/18] test(no-container): define scenarios --- tests/lib/rules/no-container.test.ts | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/lib/rules/no-container.test.ts diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts new file mode 100644 index 00000000..75948109 --- /dev/null +++ b/tests/lib/rules/no-container.test.ts @@ -0,0 +1,66 @@ +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() + `, + }, + ], + 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', + }, + ], + }, + ], +}); From 5d02426447ae9540791ccc11f82cacda3b611736 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 10:55:02 -0300 Subject: [PATCH 04/18] refactor(no-debug): extract auxiliary functions to node-utils, to be used by no-container --- lib/node-utils.ts | 33 +++++++++++++++++++++++++++++++++ lib/rules/no-container.ts | 16 +++------------- lib/rules/no-debug.ts | 35 +---------------------------------- 3 files changed, 37 insertions(+), 47 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 53e788b8..271ea0dd 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -105,4 +105,37 @@ export function hasThenProperty(node: TSESTree.Node) { export function isArrowFunctionExpression(node: TSESTree.Node): 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; } \ No newline at end of file diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 7788c82c..073df401 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -6,22 +6,11 @@ import { isMemberExpression, isObjectPattern, isProperty, + isRenderVariableDeclarator, } from '../node-utils'; export const RULE_NAME = 'no-container'; -function isRender(callNode: TSESTree.CallExpression) { - return isIdentifier(callNode.callee) && callNode.callee.name === 'render'; -} - -function isRenderVariableDeclarator(node: TSESTree.VariableDeclarator) { - if (node.init) { - if (isCallExpression(node.init)) { - return isRender(node.init); - } - } -} - export default ESLintUtils.RuleCreator(getDocsUrl)({ name: RULE_NAME, meta: { @@ -32,7 +21,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ recommended: 'error', }, messages: { - noContainer: 'Unexpected use of container methods. Prefer the use of "screen.someMethod()".', + noContainer: + 'Unexpected use of container methods. Prefer the use of "screen.someMethod()".', }, fixable: null, schema: [], 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 ) { From 020537493d5d097cc980c3cdc89619a192357ef3 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 12:55:18 -0300 Subject: [PATCH 05/18] refactor(no-container): add conditional for containerIndex | clean up method declaration --- lib/rules/no-container.ts | 13 +++++++------ tests/lib/rules/no-container.test.ts | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 073df401..685d2f8c 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -42,16 +42,17 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ isIdentifier(property.key) && property.key.name === 'container' ); - const nodeValue = node.id.properties[containerIndex].value; - destructuredContainerName = - containerIndex !== -1 && - isIdentifier(nodeValue) && - nodeValue.name; + if (containerIndex !== -1) { + const nodeValue = node.id.properties[containerIndex].value; + destructuredContainerName = + isIdentifier(nodeValue) && + nodeValue.name; + } } } }, - [`CallExpression`](node: TSESTree.CallExpression) { + CallExpression(node: TSESTree.CallExpression) { if ( isMemberExpression(node.callee) && isIdentifier(node.callee.object) && diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index 75948109..696d0133 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, { invalid: [ { code: ` - const {container} = render() + const { container } = render() const button = container.querySelector('.btn-primary') `, errors: [ From d8e555eac8755e64db32aa044c62037c78ac3055 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 13:14:30 -0300 Subject: [PATCH 06/18] docs(no-container): add new and update README --- README.md | 4 ++++ docs/rules/no-container.md | 37 +++++++++++++++++++++++++++++++++++++ lib/rules/no-container.ts | 2 +- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-container.md diff --git a/README.md b/README.md index 806db2a0..68450f4e 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-26-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -138,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 | ![recommended-badge][] ![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` | | | @@ -214,6 +217,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-container.md b/docs/rules/no-container.md new file mode 100644 index 00000000..51c7ec94 --- /dev/null +++ b/docs/rules/no-container.md @@ -0,0 +1,37 @@ +# 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. + +## 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'); +``` + +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 + +- [debug API in React Testing Library](https://testing-library.com/docs/react-testing-library/api#debug) +- [`screen.debug` in Dom Testing Library](https://testing-library.com/docs/dom-testing-library/api-queries#screendebug) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 685d2f8c..ce415eec 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -16,7 +16,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ meta: { type: 'problem', docs: { - description: 'Disallow the usage of container methods', + description: 'Disallow the use of container methods', category: 'Best Practices', recommended: 'error', }, From 497a5b602e9e8a9cc32ab8bf06ecebec1efe2f55 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 13:18:51 -0300 Subject: [PATCH 07/18] refactor(no-container): allow custom render functions --- lib/node-utils.ts | 2 +- lib/rules/no-container.ts | 25 +++++++++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 271ea0dd..b3907a00 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -118,7 +118,7 @@ function isRenderFunction( export function isRenderVariableDeclarator( node: TSESTree.VariableDeclarator, - renderFunctions: string[] = [] + renderFunctions: string[] ) { if (node.init) { if (isAwaitExpression(node.init)) { diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index ce415eec..a290373b 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -25,16 +25,30 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ 'Unexpected use of container methods. Prefer the use of "screen.someMethod()".', }, fixable: null, - schema: [], + schema: [ + { + type: 'object', + properties: { + renderFunctions: { + type: 'array', + }, + }, + }, + ], }, - defaultOptions: [], + defaultOptions: [ + { + renderFunctions: [], + }, + ], - create(context) { + create(context, [options]) { + const { renderFunctions } = options; let destructuredContainerName = ''; return { VariableDeclarator(node) { - if (isRenderVariableDeclarator(node)) { + if (isRenderVariableDeclarator(node, renderFunctions)) { if (isObjectPattern(node.id)) { const containerIndex = node.id.properties.findIndex( property => @@ -45,8 +59,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ if (containerIndex !== -1) { const nodeValue = node.id.properties[containerIndex].value; destructuredContainerName = - isIdentifier(nodeValue) && - nodeValue.name; + isIdentifier(nodeValue) && nodeValue.name; } } } From 4ab2305211c823c904c05475d16896a6c7db3e92 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 13:22:30 -0300 Subject: [PATCH 08/18] test(no-container): add custom render function --- tests/lib/rules/no-container.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index 696d0133..e9f48422 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -62,5 +62,21 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + code: ` + const { container } = renderWithRedux() + container.querySelector() + `, + options: [ + { + renderFunctions: ['renderWithRedux'], + }, + ], + errors: [ + { + messageId: 'noContainer', + }, + ], + }, ], }); From c11fcd66201c6079553d3b0115edad9ad9215652 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 13:28:06 -0300 Subject: [PATCH 09/18] docs(no-container): update further reading topics --- docs/rules/no-container.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md index 51c7ec94..25c9fe0c 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -33,5 +33,4 @@ If you use [custom render functions](https://testing-library.com/docs/example-re ## Further Reading -- [debug API in React Testing Library](https://testing-library.com/docs/react-testing-library/api#debug) -- [`screen.debug` in Dom Testing Library](https://testing-library.com/docs/dom-testing-library/api-queries#screendebug) +- [querying with `screen`](https://testing-library.com/docs/dom-testing-library/api-queries#screen) From 1c591220352c8f456ec09f656b24b59b2dfcacb9 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 16:23:25 -0300 Subject: [PATCH 10/18] docs(no-container): update | add description about testing library frameworks | add link to container docs | remove recommended badge --- README.md | 2 +- docs/rules/no-container.md | 3 +++ lib/index.ts | 4 +++- tests/__snapshots__/index.test.ts.snap | 1 - 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 31dcff42..77013b0b 100644 --- a/README.md +++ b/README.md @@ -140,7 +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 | ![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 index 25c9fe0c..79227666 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -2,6 +2,8 @@ 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. @@ -33,4 +35,5 @@ If you use [custom render functions](https://testing-library.com/docs/example-re ## 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 eec0de84..39a029ef 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -36,7 +36,6 @@ const recommendedRules = { '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-wait-for-empty-callback': 'error', 'testing-library/prefer-find-by': 'error', 'testing-library/prefer-screen-queries': 'error', @@ -53,6 +52,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 +61,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 +71,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/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index d03bf75d..f6962d95 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -53,7 +53,6 @@ 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-wait-for-empty-callback": "error", "testing-library/prefer-find-by": "error", "testing-library/prefer-screen-queries": "error", From 52f59952f24969061cd92b3f74e450661020e868 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 18:28:23 -0300 Subject: [PATCH 11/18] test(no-container): add scenarios --- tests/lib/rules/no-container.test.ts | 68 ++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index e9f48422..e3855128 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -11,28 +11,69 @@ ruleTester.run(RULE_NAME, rule, { valid: [ { code: ` - render() - screen.getByRole('button', {name: /click me/i}) + render(); + screen.getByRole('button', {name: /click me/i}); `, }, { code: ` - const { container } = render() - expect(container.firstChild).toBeDefined() + const { container } = render(); + expect(container.firstChild).toBeDefined(); `, }, { code: ` - const { container: alias } = render() - expect(alias.firstChild).toBeDefined() + 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(); `, }, ], invalid: [ { code: ` - const { container } = render() - const button = container.querySelector('.btn-primary') + 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: [ { @@ -42,8 +83,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - const { container } = render() - container.querySelector() + const button = screen.container.querySelector('.btn-primary') `, errors: [ { @@ -53,8 +93,8 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - const { container: alias } = render() - alias.querySelector() + const view = render() + const button = view.container.querySelector('.btn-primary') `, errors: [ { @@ -64,8 +104,8 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - const { container } = renderWithRedux() - container.querySelector() + const { container } = renderWithRedux(); + container.querySelector(); `, options: [ { From 4eb5381612b468f481b5177c06af9bbdb24375ae Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 18:29:55 -0300 Subject: [PATCH 12/18] refactor(no-container): adjust to new scenarios | update error message --- lib/rules/no-container.ts | 53 +++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index a290373b..6a1362d6 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -1,7 +1,6 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { getDocsUrl } from '../utils'; import { - isCallExpression, isIdentifier, isMemberExpression, isObjectPattern, @@ -22,7 +21,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, messages: { noContainer: - 'Unexpected use of container methods. Prefer the use of "screen.someMethod()".', + 'Avoid using container to query for elements. Prefer using query methods from Testing Library, such as "getByRole()"', }, fixable: null, schema: [ @@ -44,7 +43,9 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ create(context, [options]) { const { renderFunctions } = options; - let destructuredContainerName = ''; + let containerName = ''; + let renderWrapperName = ''; + let hasPropertyContainer = false; return { VariableDeclarator(node) { @@ -56,25 +57,45 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ isIdentifier(property.key) && property.key.name === 'container' ); - if (containerIndex !== -1) { - const nodeValue = node.id.properties[containerIndex].value; - destructuredContainerName = - isIdentifier(nodeValue) && nodeValue.name; - } + const nodeValue = + containerIndex !== -1 && node.id.properties[containerIndex].value; + containerName = isIdentifier(nodeValue) && nodeValue.name; + } else { + renderWrapperName = isIdentifier(node.id) && node.id.name; } } }, CallExpression(node: TSESTree.CallExpression) { - if ( - isMemberExpression(node.callee) && - isIdentifier(node.callee.object) && - node.callee.object.name === destructuredContainerName + function showErrorForChainedContainerMethod( + innerNode: TSESTree.MemberExpression ) { - context.report({ - node, - messageId: 'noContainer', - }); + if (isMemberExpression(innerNode)) { + if (isIdentifier(innerNode.object)) { + const isScreen = innerNode.object.name === 'screen'; + const isContainerName = innerNode.object.name === containerName; + const isRenderWrapper = + innerNode.object.name === renderWrapperName; + + hasPropertyContainer = + isIdentifier(innerNode.property) && + innerNode.property.name === 'container' && + (isScreen || isRenderWrapper); + + if (isContainerName || hasPropertyContainer) { + context.report({ + node, + messageId: 'noContainer', + }); + } + } + showErrorForChainedContainerMethod( + innerNode.object as TSESTree.MemberExpression + ); + } + } + if (isMemberExpression(node.callee)) { + showErrorForChainedContainerMethod(node.callee); } }, }; From 3943f45705ca5a21234d3af18b8e5e54519f5462 Mon Sep 17 00:00:00 2001 From: Mateus Felix Date: Fri, 19 Jun 2020 18:45:42 -0300 Subject: [PATCH 13/18] docs(no-container): add incorrect use cases --- docs/rules/no-container.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md index 79227666..6131a3ad 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -20,6 +20,15 @@ const { container: alias } = render(); const button = alias.querySelector('.btn-primary'); ``` +```js +const button = screen.container.querySelector('.btn-primary'); +``` + +```js +const view = render(); +const button = view.container.querySelector('.btn-primary'); +``` + Examples of **correct** code for this rule: ```js @@ -30,7 +39,7 @@ 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"]}], +"testing-library/no-container": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}], ``` ## Further Reading From 8e2cdcce37b05a274f44f7d5bd26e0983f8eb52c Mon Sep 17 00:00:00 2001 From: thebinaryfelix Date: Sat, 20 Jun 2020 13:05:15 -0300 Subject: [PATCH 14/18] refactor(no-container): remove wrong use case scenario --- docs/rules/no-container.md | 4 ---- lib/rules/no-container.ts | 3 +-- tests/lib/rules/no-container.test.ts | 10 ---------- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md index 6131a3ad..952428fa 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -20,10 +20,6 @@ const { container: alias } = render(); const button = alias.querySelector('.btn-primary'); ``` -```js -const button = screen.container.querySelector('.btn-primary'); -``` - ```js const view = render(); const button = view.container.querySelector('.btn-primary'); diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 6a1362d6..da233d05 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -72,7 +72,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ ) { if (isMemberExpression(innerNode)) { if (isIdentifier(innerNode.object)) { - const isScreen = innerNode.object.name === 'screen'; const isContainerName = innerNode.object.name === containerName; const isRenderWrapper = innerNode.object.name === renderWrapperName; @@ -80,7 +79,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ hasPropertyContainer = isIdentifier(innerNode.property) && innerNode.property.name === 'container' && - (isScreen || isRenderWrapper); + isRenderWrapper; if (isContainerName || hasPropertyContainer) { context.report({ diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index e3855128..879b42f5 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -81,16 +81,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - { - code: ` - const button = screen.container.querySelector('.btn-primary') - `, - errors: [ - { - messageId: 'noContainer', - }, - ], - }, { code: ` const view = render() From 78fdfe74d3b9c95196b6e41013e155d8835d216f Mon Sep 17 00:00:00 2001 From: thebinaryfelix Date: Sat, 20 Jun 2020 15:42:46 -0300 Subject: [PATCH 15/18] refactor(no-container): add scenario | destructure method from container --- lib/rules/no-container.ts | 25 +++++++++++++++++++++---- tests/lib/rules/no-container.test.ts | 23 ++++++++++++++++++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index da233d05..89bb292d 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -43,9 +43,10 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ create(context, [options]) { const { renderFunctions } = options; - let containerName = ''; - let renderWrapperName = ''; let hasPropertyContainer = false; + let containerName: string = null; + let renderWrapperName: string = null; + const destructuredContainerPropNames: string[] = []; return { VariableDeclarator(node) { @@ -59,7 +60,17 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ ); const nodeValue = containerIndex !== -1 && node.id.properties[containerIndex].value; - containerName = isIdentifier(nodeValue) && nodeValue.name; + 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; } @@ -83,7 +94,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ if (isContainerName || hasPropertyContainer) { context.report({ - node, + node: innerNode, messageId: 'noContainer', }); } @@ -95,6 +106,12 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } if (isMemberExpression(node.callee)) { showErrorForChainedContainerMethod(node.callee); + } else if (isIdentifier(node.callee)) { + destructuredContainerPropNames.includes(node.callee.name) && + context.report({ + node, + messageId: 'noContainer', + }); } }, }; diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index 879b42f5..a6a6116a 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -37,7 +37,7 @@ ruleTester.run(RULE_NAME, rule, { \`; const button = container.querySelector('button'); - + button.addEventListener('click', () => console.log('clicked')); return container; } @@ -46,6 +46,12 @@ ruleTester.run(RULE_NAME, rule, { screen.getByText(exampleDOM, 'Print Username').click(); `, }, + { + code: ` + const { container: { firstChild } } = render(); + expect(firstChild).toBeDefined(); + `, + }, ], invalid: [ { @@ -83,8 +89,19 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - const view = render() - const button = view.container.querySelector('.btn-primary') + const view = render(); + const button = view.container.querySelector('.btn-primary'); + `, + errors: [ + { + messageId: 'noContainer', + }, + ], + }, + { + code: ` + const { container: { querySelector } } = render(); + querySelector('foo'); `, errors: [ { From 9812cf4dcbd6b93f73ff37c047dd2d0e14879b69 Mon Sep 17 00:00:00 2001 From: thebinaryfelix Date: Sat, 20 Jun 2020 16:16:21 -0300 Subject: [PATCH 16/18] refactor(no-container): rename function and change its scope --- lib/rules/no-container.ts | 60 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 89bb292d..56f941f3 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -43,10 +43,36 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ create(context, [options]) { const { renderFunctions } = options; - let hasPropertyContainer = false; - let containerName: string = null; - let renderWrapperName: string = null; 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) { @@ -78,34 +104,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, CallExpression(node: TSESTree.CallExpression) { - function showErrorForChainedContainerMethod( - innerNode: TSESTree.MemberExpression - ) { - if (isMemberExpression(innerNode)) { - if (isIdentifier(innerNode.object)) { - const isContainerName = innerNode.object.name === containerName; - const isRenderWrapper = - innerNode.object.name === renderWrapperName; - - hasPropertyContainer = - isIdentifier(innerNode.property) && - innerNode.property.name === 'container' && - isRenderWrapper; - - if (isContainerName || hasPropertyContainer) { - context.report({ - node: innerNode, - messageId: 'noContainer', - }); - } - } - showErrorForChainedContainerMethod( - innerNode.object as TSESTree.MemberExpression - ); - } - } if (isMemberExpression(node.callee)) { - showErrorForChainedContainerMethod(node.callee); + showErrorIfChainedContainerMethod(node.callee); } else if (isIdentifier(node.callee)) { destructuredContainerPropNames.includes(node.callee.name) && context.report({ From e63968f6e7039ede21b82378423580443bc42844 Mon Sep 17 00:00:00 2001 From: thebinaryfelix Date: Sat, 20 Jun 2020 16:34:40 -0300 Subject: [PATCH 17/18] refactor(no-container): remove condition --- lib/rules/no-container.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 56f941f3..5f0450bc 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -106,8 +106,9 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ CallExpression(node: TSESTree.CallExpression) { if (isMemberExpression(node.callee)) { showErrorIfChainedContainerMethod(node.callee); - } else if (isIdentifier(node.callee)) { - destructuredContainerPropNames.includes(node.callee.name) && + } else { + isIdentifier(node.callee) && + destructuredContainerPropNames.includes(node.callee.name) && context.report({ node, messageId: 'noContainer', From 5aa3a8059a52463cd3d36a372848b61a12fcbf6e Mon Sep 17 00:00:00 2001 From: thebinaryfelix Date: Sat, 20 Jun 2020 17:47:07 -0300 Subject: [PATCH 18/18] refactor(no-container): update error message and incorrect use case --- docs/rules/no-container.md | 2 +- lib/rules/no-container.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md index 952428fa..658879e2 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -22,7 +22,7 @@ const button = alias.querySelector('.btn-primary'); ```js const view = render(); -const button = view.container.querySelector('.btn-primary'); +const button = view.container.getElementsByClassName('.btn-primary'); ``` Examples of **correct** code for this rule: diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 5f0450bc..cd7c67a9 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -21,7 +21,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, messages: { noContainer: - 'Avoid using container to query for elements. Prefer using query methods from Testing Library, such as "getByRole()"', + 'Avoid using container methods. Prefer using the methods from Testing Library, such as "getByRole()"', }, fixable: null, schema: [