-
Notifications
You must be signed in to change notification settings - Fork 149
feat: add no-container rule #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+336
−34
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
094f53c
feat(no-container): setup new rule on index
da88a9d
refactor(no-container): set rules
ee369e5
test(no-container): define scenarios
5d02426
refactor(no-debug): extract auxiliary functions
0205374
refactor(no-container): add conditional
d8e555e
docs(no-container): add new and update README
497a5b6
refactor(no-container): allow custom render
4ab2305
test(no-container): add custom render function
c11fcd6
docs(no-container): update further reading topics
1d8077c
Merge branch 'v4' of https://github.com/thebinaryfelix/eslint-plugin-…
d624bfe
Merge branch 'v4' into no-container
thebinaryfelix 1c59122
docs(no-container): update
52f5995
test(no-container): add scenarios
4eb5381
refactor(no-container): adjust to new scenarios
3943f45
docs(no-container): add incorrect use cases
8e2cdcc
refactor(no-container): remove wrong use case
thebinaryfelix 78fdfe7
refactor(no-container): add scenario
thebinaryfelix 9812cf4
refactor(no-container): rename function
thebinaryfelix e63968f
refactor(no-container): remove condition
thebinaryfelix 5aa3a80
refactor(no-container): update error message
thebinaryfelix 9d44911
Merge branch 'v4' into no-container
thebinaryfelix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(<Example />); | ||
const button = container.querySelector('.btn-primary'); | ||
``` | ||
|
||
```js | ||
const { container: alias } = render(<Example />); | ||
const button = alias.querySelector('.btn-primary'); | ||
``` | ||
thebinaryfelix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```js | ||
const view = render(<Example />); | ||
const button = view.container.getElementsByClassName('.btn-primary'); | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```js | ||
render(<Example />); | ||
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) | ||
thebinaryfelix marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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', | ||
}); | ||
} | ||
}, | ||
}; | ||
}, | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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: [ | ||
thebinaryfelix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
code: ` | ||
render(<Example />); | ||
screen.getByRole('button', {name: /click me/i}); | ||
`, | ||
}, | ||
{ | ||
code: ` | ||
const { container } = render(<Example />); | ||
expect(container.firstChild).toBeDefined(); | ||
`, | ||
}, | ||
{ | ||
code: ` | ||
const { container: alias } = render(<Example />); | ||
expect(alias.firstChild).toBeDefined(); | ||
`, | ||
}, | ||
{ | ||
code: ` | ||
function getExampleDOM() { | ||
const container = document.createElement('div'); | ||
container.innerHTML = \` | ||
<label for="username">Username</label> | ||
<input id="username" /> | ||
<button>Print Username</button> | ||
\`; | ||
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(<Example />); | ||
expect(firstChild).toBeDefined(); | ||
`, | ||
}, | ||
], | ||
invalid: [ | ||
{ | ||
code: ` | ||
const { container } = render(<Example />); | ||
const button = container.querySelector('.btn-primary'); | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const { container } = render(<Example />); | ||
container.querySelector(); | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const { container: alias } = render(<Example />); | ||
alias.querySelector(); | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const view = render(<Example />); | ||
const button = view.container.querySelector('.btn-primary'); | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const { container: { querySelector } } = render(<Example />); | ||
querySelector('foo'); | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
const { container } = renderWithRedux(<Example />); | ||
container.querySelector(); | ||
`, | ||
options: [ | ||
{ | ||
renderFunctions: ['renderWithRedux'], | ||
}, | ||
], | ||
errors: [ | ||
{ | ||
messageId: 'noContainer', | ||
}, | ||
], | ||
}, | ||
], | ||
thebinaryfelix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.