Skip to content
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

feat: add prefer-query-matchers rule #750

Merged
merged 9 commits into from
May 10, 2023
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ module.exports = {
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `find(All)By*` query instead of `waitFor` + `get(All)By*` to wait for elements | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | 🔧 |
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Ensure appropriate `get*`/`query*` queries are used with their respective matchers | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | |
| [prefer-query-by-disappearance](docs/rules/prefer-query-by-disappearance.md) | Suggest using `queryBy*` queries when waiting for disappearance | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | |
| [prefer-query-matchers](docs/rules/prefer-query-matchers.md) | Ensure the configured `get*`/`query*` query is used with the corresponding matchers | | |
| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using `screen` while querying | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | |
| [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` over `fireEvent` for simulating user interactions | | |
| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | 🔧 |
Expand Down
1 change: 1 addition & 0 deletions docs/rules/prefer-presence-queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Examples of **correct** code for this rule:
```js
test('some test', async () => {
render(<App />);

// check element is present with `getBy*`
expect(screen.getByText('button')).toBeInTheDocument();
expect(screen.getAllByText('button')[9]).toBeTruthy();
Expand Down
85 changes: 85 additions & 0 deletions docs/rules/prefer-query-matchers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Ensure the configured `get*`/`query*` query is used with the corresponding matchers (`testing-library/prefer-query-matchers`)

<!-- end auto-generated rule header -->

The (DOM) Testing Library allows to query DOM elements using different types of queries such as `get*` and `query*`. Using `get*` throws an error in case the element is not found, while `query*` returns null instead of throwing (or empty array for `queryAllBy*` ones).

It may be helpful to ensure that either `get*` or `query*` are always used for a given matcher. For example, `.toBeVisible()` and the negation `.not.toBeVisible()` both assume that an element exists in the DOM and will error if not. Using `get*` with `.toBeVisible()` ensures that if the element is not found the error thrown will offer better info than with `query*`.

## Rule details

This rule must be configured with a list of `validEntries`: for a given matcher, is `get*` or `query*` required.

Assuming the following configuration:

```json
{
"testing-library/prefer-query-matchers": [
2,
{
"validEntries": [{ "matcher": "toBeVisible", "query": "get" }]
}
]
}
```

Examples of **incorrect** code for this rule with the above configuration:

```js
test('some test', () => {
render(<App />);

// use configured matcher with the disallowed `query*`
expect(screen.queryByText('button')).toBeVisible();
expect(screen.queryByText('button')).not.toBeVisible();
expect(screen.queryAllByText('button')[0]).toBeVisible();
expect(screen.queryAllByText('button')[0]).not.toBeVisible();
});
```

Examples of **correct** code for this rule:

```js
test('some test', async () => {
render(<App />);
// use configured matcher with the allowed `get*`
expect(screen.getByText('button')).toBeVisible();
expect(screen.getByText('button')).not.toBeVisible();
expect(screen.getAllByText('button')[0]).toBeVisible();
expect(screen.getAllByText('button')[0]).not.toBeVisible();

// use an unconfigured matcher with either `get* or `query*
expect(screen.getByText('button')).toBeEnabled();
expect(screen.getAllByText('checkbox')[0]).not.toBeChecked();
expect(screen.queryByText('button')).toHaveFocus();
expect(screen.queryAllByText('button')[0]).not.toMatchMyCustomMatcher();

// `findBy*` queries are out of the scope for this rule
const button = await screen.findByText('submit');
expect(button).toBeVisible();
});
```

## Options

| Option | Required | Default | Details |
| -------------- | -------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `validEntries` | No | `[]` | A list of objects with a `matcher` property (the name of any matcher, such as "toBeVisible") and a `query` property (either "get" or "query"). Indicates whether `get*` or `query*` are allowed with this matcher. |

## Example

```json
{
"testing-library/prefer-query-matchers": [
2,
{
"validEntries": [{ "matcher": "toBeVisible", "query": "get" }]
}
]
}
```

## Further Reading

- [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries)
- [jest-dom note about using `getBy` within assertions](https://testing-library.com/docs/ecosystem-jest-dom)
16 changes: 16 additions & 0 deletions lib/create-testing-library-rule/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ type IsDebugUtilFn = (
validNames?: ReadonlyArray<(typeof DEBUG_UTILS)[number]>
) => boolean;
type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
type IsMatchingAssertFn = (
node: TSESTree.MemberExpression,
matcherName: string
) => boolean;
type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
type CanReportErrorsFn = () => boolean;
type FindImportedTestingLibraryUtilSpecifierFn = (
Expand Down Expand Up @@ -122,6 +126,7 @@ export interface DetectionHelpers {
isActUtil: (node: TSESTree.Identifier) => boolean;
isPresenceAssert: IsPresenceAssertFn;
isAbsenceAssert: IsAbsenceAssertFn;
isMatchingAssert: IsMatchingAssertFn;
canReportErrors: CanReportErrorsFn;
findImportedTestingLibraryUtilSpecifier: FindImportedTestingLibraryUtilSpecifierFn;
isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn;
Expand Down Expand Up @@ -819,6 +824,16 @@ export function detectTestingLibraryUtils<
: ABSENCE_MATCHERS.includes(matcher);
};

const isMatchingAssert: IsMatchingAssertFn = (node, matcherName) => {
Belco90 marked this conversation as resolved.
Show resolved Hide resolved
const { matcher } = getAssertNodeInfo(node);

if (!matcher) {
return false;
}

return matcher === matcherName;
};

/**
* Finds the import util specifier related to Testing Library for a given name.
*/
Expand Down Expand Up @@ -977,6 +992,7 @@ export function detectTestingLibraryUtils<
isDebugUtil,
isActUtil,
isPresenceAssert,
isMatchingAssert,
isAbsenceAssert,
canReportErrors,
findImportedTestingLibraryUtilSpecifier,
Expand Down
105 changes: 105 additions & 0 deletions lib/rules/prefer-query-matchers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { TSESTree } from '@typescript-eslint/utils';

import { createTestingLibraryRule } from '../create-testing-library-rule';
import { findClosestCallNode, isMemberExpression } from '../node-utils';

export const RULE_NAME = 'prefer-query-matchers';
export type MessageIds = 'wrongQueryForMatcher';
export type Options = [
{
validEntries: {
query: 'get' | 'query';
matcher: string;
}[];
}
];

export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
docs: {
description:
'Ensure the configured `get*`/`query*` query is used with the corresponding matchers',
recommendedConfig: {
dom: false,
angular: false,
react: false,
vue: false,
marko: false,
},
},
messages: {
wrongQueryForMatcher: 'Use `{{ query }}By*` queries for {{ matcher }}',
},
schema: [
{
type: 'object',
additionalProperties: false,
properties: {
validEntries: {
type: 'array',
items: {
type: 'object',
properties: {
query: {
type: 'string',
enum: ['get', 'query'],
},
matcher: {
type: 'string',
},
},
},
},
},
},
],
type: 'suggestion',
},
defaultOptions: [
{
validEntries: [],
},
],

create(context, [{ validEntries }], helpers) {
return {
'CallExpression Identifier'(node: TSESTree.Identifier) {
const expectCallNode = findClosestCallNode(node, 'expect');

if (!expectCallNode || !isMemberExpression(expectCallNode.parent)) {
return;
}

// Sync queries (getBy and queryBy) and corresponding ones
// are supported. If none found, stop the rule.
if (!helpers.isSyncQuery(node)) {
return;
}

const isGetBy = helpers.isGetQueryVariant(node);
const expectStatement = expectCallNode.parent;
for (const entry of validEntries) {
const { query, matcher } = entry;
const isMatchingAssertForThisEntry = helpers.isMatchingAssert(
expectStatement,
matcher
);

if (!isMatchingAssertForThisEntry) {
continue;
}

const actualQuery = isGetBy ? 'get' : 'query';
if (query !== actualQuery) {
context.report({
node,
messageId: 'wrongQueryForMatcher',
data: { query, matcher },
});
}
}
},
};
},
});
2 changes: 1 addition & 1 deletion tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { resolve } from 'path';

import plugin from '../lib';

const numberOfRules = 27;
const numberOfRules = 28;
const ruleNames = Object.keys(plugin.rules);

// eslint-disable-next-line jest/expect-expect
Expand Down
Loading