-
Notifications
You must be signed in to change notification settings - Fork 41
feat: added new rule prefer-in-document #95
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
benmonro
merged 18 commits into
testing-library:master
from
AntonNiklasson:an/feat/prefer-in-document
Nov 24, 2020
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
4d35d54
add a first rough draft of prefer-in-document
AntonNiklasson 86b23c7
add docs for the new rule
AntonNiklasson 88cd02c
make the rule fixable
AntonNiklasson 8dccaa8
support destructed query calls
AntonNiklasson 4bf9fc1
pull in query names from @testing-library/dom
AntonNiklasson 6e9ccd2
npm run generate-readme-table
AntonNiklasson 9336aae
all valid cases should be strings
AntonNiklasson 525b16c
split queries by variant, add invalid cases for query*
AntonNiklasson bd8d2a1
implement the code to pass the failing tests
AntonNiklasson 0fad7d5
update the rule docs
AntonNiklasson 4e2bc4e
get rid of Array.flat() to support node 10
AntonNiklasson 60e64a7
remove unnecessary if statement to get 100% coverage
AntonNiklasson 246d776
use node.range[0] instead of node.start for eslint@7
AntonNiklasson ddd8f9b
Merge branch 'master' into an/feat/prefer-in-document
benmonro 0d4244a
updated readme
benmonro 11dd5e7
update the rule description and reported message
AntonNiklasson 3d2333e
move DTL to deps
benmonro d182646
Merge branch 'master' into an/feat/prefer-in-document
benmonro 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,40 @@ | ||
| # Prefer .toBeInTheDocument in favor of .toHaveLength(1) (prefer-in-document) | ||
|
|
||
| ## Rule Details | ||
|
|
||
| This rule enforces checking existance of DOM nodes using `.toBeInTheDocument()`. | ||
| The rule prefers that matcher over various existance checks such as `.toHaveLength(1)`, `.not.toBeNull()` and | ||
| similar. | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```js | ||
| expect(screen.queryByText("foo")).toHaveLength(1); | ||
| expect(queryByText("foo")).toHaveLength(1); | ||
| expect(wrapper.queryByText("foo")).toHaveLength(1); | ||
| expect(queryByText("foo")).toHaveLength(0); | ||
| expect(queryByText("foo")).toBeNull(); | ||
| expect(queryByText("foo")).not.toBeNull(); | ||
| expect(queryByText("foo")).toBeDefined(); | ||
| expect(queryByText("foo")).not.toBeDefined(); | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```js | ||
| expect(screen.queryByText("foo")).toBeInTheDocument(); | ||
| expect(screen.queryByText("foo")).toBeInTheDocument(); | ||
| expect(queryByText("foo")).toBeInTheDocument()`; | ||
| expect(wrapper.queryAllByTestId('foo')).toBeInTheDocument()`; | ||
| expect(screen.getAllByLabel("foo-bar")).toHaveLength(2)`; | ||
| expect(notAQuery('foo-bar')).toHaveLength(1)`; | ||
| ``` | ||
|
|
||
| ## When Not To Use It | ||
|
|
||
| Don't use this rule if you don't care about the added readability and | ||
| improvements that `toBeInTheDocument` offers to your expects. | ||
|
|
||
| ## Further Reading | ||
|
|
||
| - [Docs on toBeInTheDocument](https://github.com/testing-library/jest-dom#tobeinthedocument) | ||
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,84 @@ | ||
| /** | ||
| * @fileoverview Prefer toBeInTheDocument over querying and asserting length. | ||
| * @author Anton Niklasson | ||
| */ | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| // Requirements | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| import { RuleTester } from "eslint"; | ||
| import { queries, queriesByVariant } from "../../../queries"; | ||
| import * as rule from "../../../rules/prefer-in-document"; | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| // Tests | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| function invalidCase(code, output) { | ||
| return { | ||
| code, | ||
| output, | ||
| errors: [ | ||
| { | ||
| messageId: "use-document", | ||
| }, | ||
| ], | ||
| }; | ||
| } | ||
|
|
||
| const valid = [ | ||
| ...queries.map((q) => [ | ||
| `expect(screen.${q}('foo')).toBeInTheDocument()`, | ||
| `expect(${q}('foo')).toBeInTheDocument()`, | ||
| `expect(wrapper.${q}('foo')).toBeInTheDocument()`, | ||
| ]), | ||
| `expect(screen.notAQuery('foo-bar')).toHaveLength(1)`, | ||
| `expect(screen.getByText('foo-bar')).toHaveLength(2)`, | ||
| ]; | ||
| const invalid = [ | ||
| // Invalid cases that applies to all variants | ||
| ...queries.map((q) => [ | ||
| invalidCase( | ||
| `expect(screen.${q}('foo')).toHaveLength(1)`, | ||
| `expect(screen.${q}('foo')).toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(${q}('foo')).toHaveLength(1)`, | ||
| `expect(${q}('foo')).toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(wrapper.${q}('foo')).toHaveLength(1)`, | ||
| `expect(wrapper.${q}('foo')).toBeInTheDocument()` | ||
| ), | ||
| ]), | ||
| // Invalid cases that applies to queryBy* and queryAllBy* | ||
| ...queriesByVariant.query.map((q) => [ | ||
| invalidCase( | ||
| `expect(${q}('foo')).toHaveLength(0)`, | ||
| `expect(${q}('foo')).not.toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(${q}('foo')).toBeNull()`, | ||
| `expect(${q}('foo')).not.toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(${q}('foo')).not.toBeNull()`, | ||
| `expect(${q}('foo')).toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(${q}('foo')).toBeDefined()`, | ||
| `expect(${q}('foo')).toBeInTheDocument()` | ||
| ), | ||
| invalidCase( | ||
| `expect(${q}('foo')).not.toBeDefined()`, | ||
| `expect(${q}('foo')).not.toBeInTheDocument()` | ||
| ), | ||
| ]), | ||
| ]; | ||
|
|
||
| const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } }); | ||
| ruleTester.run("prefer-in-document", rule, { | ||
| valid: [].concat(...valid), | ||
| invalid: [].concat(...invalid), | ||
| }); |
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,9 @@ | ||
| import { queries as allQueries } from "@testing-library/dom"; | ||
|
|
||
| export const queries = Object.keys(allQueries); | ||
|
|
||
| export const queriesByVariant = { | ||
| query: queries.filter((q) => q.startsWith("query")), | ||
| get: queries.filter((q) => q.startsWith("get")), | ||
| find: queries.filter((q) => q.startsWith("find")), | ||
| }; |
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,113 @@ | ||
| /** | ||
| * @fileoverview prefer toBeInTheDocument over checking getAttribute/hasAttribute | ||
| * @author Anton Niklasson | ||
| */ | ||
|
|
||
| import { queries } from "../queries"; | ||
|
|
||
| export const meta = { | ||
| type: "suggestion", | ||
| docs: { | ||
| category: "jest-dom", | ||
| description: | ||
| "Prefer .toBeInTheDocument() for asserting the existence of a DOM node", | ||
| url: "prefer-in-document", | ||
| recommended: false, | ||
| }, | ||
| fixable: "code", | ||
| messages: { | ||
| "use-document": `Prefer .toBeInTheDocument() for asserting DOM node existence`, | ||
| }, | ||
| }; | ||
|
|
||
| function isAntonymMatcher(matcherNode, matcherArguments) { | ||
| return ( | ||
| matcherNode.name === "toBeNull" || | ||
| (matcherNode.name === "toHaveLength" && matcherArguments[0].value === 0) | ||
| ); | ||
| } | ||
|
|
||
| function check( | ||
| context, | ||
| { queryNode, matcherNode, matcherArguments, negatedMatcher } | ||
| ) { | ||
| const query = queryNode.name || queryNode.property.name; | ||
|
|
||
| // toHaveLength() is only invalid with 0 or 1 | ||
| if (matcherNode.name === "toHaveLength" && matcherArguments[0].value > 1) { | ||
| return; | ||
| } | ||
|
|
||
| if (queries.includes(query)) { | ||
| context.report({ | ||
| node: matcherNode, | ||
| messageId: "use-document", | ||
| loc: matcherNode.loc, | ||
| fix(fixer) { | ||
| const operations = []; | ||
|
|
||
| // Flip the .not if neccessary | ||
| if (isAntonymMatcher(matcherNode, matcherArguments)) { | ||
| if (negatedMatcher) { | ||
| operations.push( | ||
| fixer.removeRange([ | ||
| matcherNode.range[0] - 5, | ||
| matcherNode.range[0] - 1, | ||
| ]) | ||
| ); | ||
| } else { | ||
| operations.push(fixer.insertTextBefore(matcherNode, "not.")); | ||
| } | ||
| } | ||
|
|
||
| // Replace the actual matcher | ||
| operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); | ||
|
|
||
| // Remove any arguments in the matcher | ||
| for (const argument of matcherArguments) { | ||
| operations.push(fixer.remove(argument)); | ||
| } | ||
|
|
||
| return operations; | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export const create = (context) => { | ||
| const alternativeMatchers = /(toHaveLength|toBeDefined|toBeNull)/; | ||
|
|
||
| return { | ||
| // Grabbing expect(<query>).not.<matcher> | ||
| [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`]( | ||
| node | ||
| ) { | ||
| const queryNode = node.callee.object.object.arguments[0].callee; | ||
| const matcherNode = node.callee.property; | ||
| const matcherArguments = node.arguments; | ||
|
|
||
| check(context, { | ||
| negatedMatcher: true, | ||
| queryNode, | ||
| matcherNode, | ||
| matcherArguments, | ||
| }); | ||
| }, | ||
|
|
||
| // Grabbing expect(<query>).<matcher> | ||
| [`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}]`]( | ||
| node | ||
| ) { | ||
| const queryNode = node.callee.object.arguments[0].callee; | ||
| const matcherNode = node.callee.property; | ||
| const matcherArguments = node.arguments; | ||
|
|
||
| check(context, { | ||
| negatedMatcher: false, | ||
| queryNode, | ||
| matcherNode, | ||
| matcherArguments, | ||
| }); | ||
| }, | ||
| }; | ||
| }; |
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.