Skip to content

Commit 3b9d5b5

Browse files
Belco90Belco90
authored
feat: add render-result-naming-convention rule (#200)
* feat: rule skeleton * test: first round * feat: rule implementation round 1 * refactor: move hasTestingLibraryImportModule * test: fix invalid lines * feat: check imported module * feat: check imported render renamed * feat: check custom render * test: add more valid tests for custom render functions * test: update tests for render wrapper functions * docs: add rule docs * test: increase coverage up to 100% * fix: add rule meta description * docs: update rule details to mention screen object * refactor: return as soon as conditions are not met * feat: check wildcard imports * refactor: rename default import * docs: include render result link
1 parent 17e3cfe commit 3b9d5b5

File tree

7 files changed

+602
-23
lines changed

7 files changed

+602
-23
lines changed

‎README.md

+21-20
Original file line numberDiff line numberDiff line change
@@ -125,27 +125,28 @@ To enable this configuration use the `extends` property in your
125125

126126
## Supported Rules
127127

128-
| Rule | Description | Configurations | Fixable |
129-
| -------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ |
130-
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
131-
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
132-
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
133-
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
134-
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
135-
| [no-container](docs/rules/no-container.md) | Disallow the use of `container` methods | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
136-
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
137-
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
138-
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
139-
| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | |
140-
| [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | |
141-
| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | |
142-
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
143-
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
144-
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
145-
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
128+
| Rule | Description | Configurations | Fixable |
129+
| -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ |
130+
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
131+
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
132+
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
133+
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
134+
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
135+
| [no-container](docs/rules/no-container.md) | Disallow the use of `container` methods | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
136+
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
137+
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
138+
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
139+
| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | |
140+
| [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
141+
| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | |
142+
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
143+
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
144+
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
145+
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
146146
| [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` library instead of `fireEvent` for simulating user interaction | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
147-
| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
148-
| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] |
147+
| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
148+
| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] |
149+
| [render-result-naming-convention](docs/rules/render-result-naming-convention.md) | Enforce a valid naming for return value from `render` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
149150

150151
[build-badge]: https://img.shields.io/travis/testing-library/eslint-plugin-testing-library?style=flat-square
151152
[build-url]: https://travis-ci.org/testing-library/eslint-plugin-testing-library

‎docs/rules/render-result-naming-convention.md

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Enforce a valid naming for return value from `render` (render-result-naming-convention)
2+
3+
> The name `wrapper` is old cruft from `enzyme` and we don't need that here. The return value from `render` is not "wrapping" anything. It's simply a collection of utilities that you should actually not often need anyway.
4+
5+
## Rule Details
6+
7+
This rule aims to ensure the return value from `render` is named properly.
8+
9+
Ideally, you should destructure the minimum utils that you need from `render`, combined with using queries from [`screen` object](https://github.com/testing-library/eslint-plugin-testing-library/blob/master/docs/rules/prefer-screen-queries.md). In case you need to save the collection of utils returned in a variable, its name should be either `view` or `utils`, as `render` is not wrapping anything: it's just returning a collection of utilities. Every other name for that variable will be considered invalid.
10+
11+
To sum up these rules, the allowed naming convention for return value from `render` is:
12+
13+
- destructuring
14+
- `view`
15+
- `utils`
16+
17+
Examples of **incorrect** code for this rule:
18+
19+
```javascript
20+
import { render } from '@testing-library/framework';
21+
22+
// ...
23+
24+
// return value from `render` shouldn't be kept in a var called "wrapper"
25+
const wrapper = render(<SomeComponent />);
26+
```
27+
28+
```javascript
29+
import { render } from '@testing-library/framework';
30+
31+
// ...
32+
33+
// return value from `render` shouldn't be kept in a var called "component"
34+
const component = render(<SomeComponent />);
35+
```
36+
37+
```javascript
38+
import { render } from '@testing-library/framework';
39+
40+
// ...
41+
42+
// to sum up: return value from `render` shouldn't be kept in a var called other than "view" or "utils"
43+
const somethingElse = render(<SomeComponent />);
44+
```
45+
46+
Examples of **correct** code for this rule:
47+
48+
```javascript
49+
import { render } from '@testing-library/framework';
50+
51+
// ...
52+
53+
// destructuring return value from `render` is correct
54+
const { unmount, rerender } = render(<SomeComponent />);
55+
```
56+
57+
```javascript
58+
import { render } from '@testing-library/framework';
59+
60+
// ...
61+
62+
// keeping return value from `render` in a var called "view" is correct
63+
const view = render(<SomeComponent />);
64+
```
65+
66+
```javascript
67+
import { render } from '@testing-library/framework';
68+
69+
// ...
70+
71+
// keeping return value from `render` in a var called "utils" is correct
72+
const utils = render(<SomeComponent />);
73+
```
74+
75+
## Further Reading
76+
77+
- [Common Mistakes with React Testing Library](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#using-wrapper-as-the-variable-name-for-the-return-value-from-render)
78+
- [`render` Result](https://testing-library.com/docs/react-testing-library/api#render-result)

‎lib/index.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import preferPresenceQueries from './rules/prefer-presence-queries';
1515
import preferScreenQueries from './rules/prefer-screen-queries';
1616
import preferUserEvent from './rules/prefer-user-event';
1717
import preferWaitFor from './rules/prefer-wait-for';
18-
import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for'
18+
import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for';
1919
import preferFindBy from './rules/prefer-find-by';
20+
import renderResultNamingConvention from './rules/render-result-naming-convention';
2021

2122
const rules = {
2223
'await-async-query': awaitAsyncQuery,
@@ -38,6 +39,7 @@ const rules = {
3839
'prefer-screen-queries': preferScreenQueries,
3940
'prefer-user-event': preferUserEvent,
4041
'prefer-wait-for': preferWaitFor,
42+
'render-result-naming-convention': renderResultNamingConvention,
4143
};
4244

4345
const domRules = {
@@ -57,6 +59,7 @@ const angularRules = {
5759
'testing-library/no-debug': 'warn',
5860
'testing-library/no-dom-import': ['error', 'angular'],
5961
'testing-library/no-node-access': 'error',
62+
'testing-library/render-result-naming-convention': 'error',
6063
};
6164

6265
const reactRules = {
@@ -65,6 +68,7 @@ const reactRules = {
6568
'testing-library/no-debug': 'warn',
6669
'testing-library/no-dom-import': ['error', 'react'],
6770
'testing-library/no-node-access': 'error',
71+
'testing-library/render-result-naming-convention': 'error',
6872
};
6973

7074
const vueRules = {
@@ -74,6 +78,7 @@ const vueRules = {
7478
'testing-library/no-debug': 'warn',
7579
'testing-library/no-dom-import': ['error', 'vue'],
7680
'testing-library/no-node-access': 'error',
81+
'testing-library/render-result-naming-convention': 'error',
7782
};
7883

7984
export = {

‎lib/rules/render-result-naming-convention.ts

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { getDocsUrl, hasTestingLibraryImportModule } from '../utils';
3+
import {
4+
isCallExpression,
5+
isIdentifier,
6+
isImportSpecifier,
7+
isMemberExpression,
8+
isObjectPattern,
9+
isRenderVariableDeclarator,
10+
} from '../node-utils';
11+
12+
export const RULE_NAME = 'render-result-naming-convention';
13+
14+
const ALLOWED_VAR_NAMES = ['view', 'utils'];
15+
const ALLOWED_VAR_NAMES_TEXT = ALLOWED_VAR_NAMES.map(
16+
name => '`' + name + '`'
17+
).join(', ');
18+
19+
export default ESLintUtils.RuleCreator(getDocsUrl)({
20+
name: RULE_NAME,
21+
meta: {
22+
type: 'suggestion',
23+
docs: {
24+
description: 'Enforce a valid naming for return value from `render`',
25+
category: 'Best Practices',
26+
recommended: false,
27+
},
28+
messages: {
29+
invalidRenderResultName: `\`{{ varName }}\` is not a recommended name for \`render\` returned value. Instead, you should destructure it, or call it using one of the valid choices: ${ALLOWED_VAR_NAMES_TEXT}`,
30+
},
31+
fixable: null,
32+
schema: [
33+
{
34+
type: 'object',
35+
properties: {
36+
renderFunctions: {
37+
type: 'array',
38+
},
39+
},
40+
},
41+
],
42+
},
43+
defaultOptions: [
44+
{
45+
renderFunctions: [],
46+
},
47+
],
48+
49+
create(context, [options]) {
50+
const { renderFunctions } = options;
51+
let renderAlias: string | undefined;
52+
let wildcardImportName: string | undefined;
53+
54+
return {
55+
// check named imports
56+
ImportDeclaration(node: TSESTree.ImportDeclaration) {
57+
if (!hasTestingLibraryImportModule(node)) {
58+
return;
59+
}
60+
const renderImport = node.specifiers.find(
61+
node => isImportSpecifier(node) && node.imported.name === 'render'
62+
);
63+
64+
if (!renderImport) {
65+
return;
66+
}
67+
68+
renderAlias = renderImport.local.name;
69+
},
70+
// check wildcard imports
71+
'ImportDeclaration ImportNamespaceSpecifier'(
72+
node: TSESTree.ImportNamespaceSpecifier
73+
) {
74+
if (
75+
!hasTestingLibraryImportModule(
76+
node.parent as TSESTree.ImportDeclaration
77+
)
78+
) {
79+
return;
80+
}
81+
82+
wildcardImportName = node.local.name;
83+
},
84+
VariableDeclarator(node: TSESTree.VariableDeclarator) {
85+
// check if destructuring return value from render
86+
if (isObjectPattern(node.id)) {
87+
return;
88+
}
89+
90+
const isValidRenderDeclarator = isRenderVariableDeclarator(node, [
91+
...renderFunctions,
92+
renderAlias,
93+
]);
94+
const isValidWildcardImport = !!wildcardImportName;
95+
96+
// check if is a Testing Library related import
97+
if (!isValidRenderDeclarator && !isValidWildcardImport) {
98+
return;
99+
}
100+
101+
const renderFunctionName =
102+
isCallExpression(node.init) &&
103+
isIdentifier(node.init.callee) &&
104+
node.init.callee.name;
105+
106+
const renderFunctionObjectName =
107+
isCallExpression(node.init) &&
108+
isMemberExpression(node.init.callee) &&
109+
isIdentifier(node.init.callee.property) &&
110+
isIdentifier(node.init.callee.object) &&
111+
node.init.callee.property.name === 'render' &&
112+
node.init.callee.object.name;
113+
114+
const isRenderAlias = !!renderAlias;
115+
const isCustomRender = renderFunctions.includes(renderFunctionName);
116+
const isWildCardRender =
117+
renderFunctionObjectName &&
118+
renderFunctionObjectName === wildcardImportName;
119+
120+
// check if is a qualified render function
121+
if (!isRenderAlias && !isCustomRender && !isWildCardRender) {
122+
return;
123+
}
124+
125+
const renderResultName = isIdentifier(node.id) && node.id.name;
126+
const isAllowedRenderResultName = ALLOWED_VAR_NAMES.includes(
127+
renderResultName
128+
);
129+
130+
// check if return value var name is allowed
131+
if (isAllowedRenderResultName) {
132+
return;
133+
}
134+
135+
context.report({
136+
node,
137+
messageId: 'invalidRenderResultName',
138+
data: {
139+
varName: renderResultName,
140+
},
141+
});
142+
},
143+
};
144+
},
145+
});

‎lib/utils.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ const LIBRARY_MODULES = [
2525
];
2626

2727
const hasTestingLibraryImportModule = (node: TSESTree.ImportDeclaration) => {
28-
return LIBRARY_MODULES.includes(node.source.value.toString())
29-
}
28+
return LIBRARY_MODULES.includes(node.source.value.toString());
29+
};
3030

3131
const SYNC_QUERIES_VARIANTS = ['getBy', 'getAllBy', 'queryBy', 'queryAllBy'];
3232
const ASYNC_QUERIES_VARIANTS = ['findBy', 'findAllBy'];

0 commit comments

Comments
 (0)