Skip to content

Commit 5f35316

Browse files
feat: new no-render-in-setup rule (#209)
* feat(no-render-in-setup): adds no-render-in-setup rule, closes #207 * refactor: address review comments * Update docs/rules/no-render-in-setup.md Co-authored-by: Tim Deschryver <[email protected]> * fix: updates docs, adds tests * fix: handle require, add tests * fix: code coverage Co-authored-by: Tim Deschryver <[email protected]>
1 parent cbdfd5f commit 5f35316

9 files changed

+464
-16
lines changed

README.md

+4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
[![Tweet][tweet-badge]][tweet-url]
2424

2525
<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
26+
2627
[![All Contributors](https://img.shields.io/badge/all_contributors-29-orange.svg?style=flat-square)](#contributors-)
28+
2729
<!-- ALL-CONTRIBUTORS-BADGE:END -->
2830

2931
## Installation
@@ -141,6 +143,7 @@ To enable this configuration use the `extends` property in your
141143
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
142144
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
143145
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
146+
| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | |
144147
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
145148
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
146149
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
@@ -219,6 +222,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
219222

220223
<!-- markdownlint-enable -->
221224
<!-- prettier-ignore-end -->
225+
222226
<!-- ALL-CONTRIBUTORS-LIST:END -->
223227

224228
This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!

docs/rules/no-render-in-setup.md

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# Disallow the use of `render` in setup functions (no-render-in-setup)
2+
3+
## Rule Details
4+
5+
This rule disallows the usage of `render` (or a custom render function) in setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions.
6+
7+
Examples of **incorrect** code for this rule:
8+
9+
```js
10+
beforeEach(() => {
11+
render(<MyComponent />);
12+
});
13+
14+
it('Should have foo', () => {
15+
expect(screen.getByText('foo')).toBeInTheDocument();
16+
});
17+
18+
it('Should have bar', () => {
19+
expect(screen.getByText('bar')).toBeInTheDocument();
20+
});
21+
```
22+
23+
```js
24+
beforeAll(() => {
25+
render(<MyComponent />);
26+
});
27+
28+
it('Should have foo', () => {
29+
expect(screen.getByText('foo')).toBeInTheDocument();
30+
});
31+
32+
it('Should have bar', () => {
33+
expect(screen.getByText('bar')).toBeInTheDocument();
34+
});
35+
```
36+
37+
Examples of **correct** code for this rule:
38+
39+
```js
40+
it('Should have foo and bar', () => {
41+
render(<MyComponent />);
42+
expect(screen.getByText('foo')).toBeInTheDocument();
43+
expect(screen.getByText('bar')).toBeInTheDocument();
44+
});
45+
```
46+
47+
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.
48+
49+
```
50+
"testing-library/no-render-in-setup": ["error", {"renderFunctions": ["renderWithRedux", "renderWithRouter"]}],
51+
```
52+
53+
If you would like to allow the use of `render` (or a custom render function) in _either_ `beforeAll` or `beforeEach`, this can be configured using the option `allowTestingFrameworkSetupHook`. This may be useful if you have configured your tests to [skip auto cleanup](https://testing-library.com/docs/react-testing-library/setup#skipping-auto-cleanup). `allowTestingFrameworkSetupHook` is an enum that accepts either `"beforeAll"` or `"beforeEach"`.
54+
55+
```
56+
"testing-library/no-render-in-setup": ["error", {"allowTestingFrameworkSetupHook": "beforeAll"}],
57+
```

lib/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import noAwaitSyncQuery from './rules/no-await-sync-query';
66
import noDebug from './rules/no-debug';
77
import noDomImport from './rules/no-dom-import';
88
import noManualCleanup from './rules/no-manual-cleanup';
9+
import noRenderInSetup from './rules/no-render-in-setup';
910
import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback';
1011
import preferExplicitAssert from './rules/prefer-explicit-assert';
1112
import preferPresenceQueries from './rules/prefer-presence-queries';
@@ -22,6 +23,7 @@ const rules = {
2223
'no-debug': noDebug,
2324
'no-dom-import': noDomImport,
2425
'no-manual-cleanup': noManualCleanup,
26+
'no-render-in-setup': noRenderInSetup,
2527
'no-wait-for-empty-callback': noWaitForEmptyCallback,
2628
'prefer-explicit-assert': preferExplicitAssert,
2729
'prefer-find-by': preferFindBy,

lib/node-utils.ts

+16
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,22 @@ export function isVariableDeclarator(
4545
return node && node.type === AST_NODE_TYPES.VariableDeclarator;
4646
}
4747

48+
export function isRenderFunction(
49+
callNode: TSESTree.CallExpression,
50+
renderFunctions: string[]
51+
) {
52+
// returns true for `render` and e.g. `customRenderFn`
53+
// as well as `someLib.render` and `someUtils.customRenderFn`
54+
return renderFunctions.some(name => {
55+
return (
56+
(isIdentifier(callNode.callee) && name === callNode.callee.name) ||
57+
(isMemberExpression(callNode.callee) &&
58+
isIdentifier(callNode.callee.property) &&
59+
name === callNode.callee.property.name)
60+
);
61+
});
62+
}
63+
4864
export function isObjectPattern(
4965
node: TSESTree.Node
5066
): node is TSESTree.ObjectPattern {

lib/rules/no-debug.ts

+6-14
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,11 @@ import {
99
isAwaitExpression,
1010
isMemberExpression,
1111
isImportSpecifier,
12+
isRenderFunction,
1213
} from '../node-utils';
1314

1415
export const RULE_NAME = 'no-debug';
1516

16-
function isRenderFunction(
17-
callNode: TSESTree.CallExpression,
18-
renderFunctions: string[]
19-
) {
20-
return ['render', ...renderFunctions].some(
21-
name => isIdentifier(callNode.callee) && name === callNode.callee.name
22-
);
23-
}
24-
2517
function isRenderVariableDeclarator(
2618
node: TSESTree.VariableDeclarator,
2719
renderFunctions: string[]
@@ -30,15 +22,15 @@ function isRenderVariableDeclarator(
3022
if (isAwaitExpression(node.init)) {
3123
return (
3224
node.init.argument &&
33-
isRenderFunction(
34-
node.init.argument as TSESTree.CallExpression,
35-
renderFunctions
36-
)
25+
isRenderFunction(node.init.argument as TSESTree.CallExpression, [
26+
'render',
27+
...renderFunctions,
28+
])
3729
);
3830
} else {
3931
return (
4032
isCallExpression(node.init) &&
41-
isRenderFunction(node.init, renderFunctions)
33+
isRenderFunction(node.init, ['render', ...renderFunctions])
4234
);
4335
}
4436
}

lib/rules/no-manual-cleanup.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
2121
meta: {
2222
type: 'problem',
2323
docs: {
24-
description: ' Disallow the use of `cleanup`',
24+
description: 'Disallow the use of `cleanup`',
2525
category: 'Best Practices',
2626
recommended: false,
2727
},
@@ -121,7 +121,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
121121
messageId: 'noManualCleanup',
122122
});
123123
}
124-
125124
} else {
126125
defaultRequireFromTestingLibrary = declaratorNode.id;
127126
}

lib/rules/no-render-in-setup.ts

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { getDocsUrl, TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils';
3+
import {
4+
isLiteral,
5+
isProperty,
6+
isIdentifier,
7+
isObjectPattern,
8+
isCallExpression,
9+
isRenderFunction,
10+
isImportSpecifier,
11+
} from '../node-utils';
12+
13+
export const RULE_NAME = 'no-render-in-setup';
14+
export type MessageIds = 'noRenderInSetup';
15+
16+
export function findClosestBeforeHook(
17+
node: TSESTree.Node,
18+
testingFrameworkSetupHooksToFilter: string[]
19+
): TSESTree.Identifier | null {
20+
if (node === null) return null;
21+
if (
22+
isCallExpression(node) &&
23+
isIdentifier(node.callee) &&
24+
testingFrameworkSetupHooksToFilter.includes(node.callee.name)
25+
) {
26+
return node.callee;
27+
}
28+
29+
return findClosestBeforeHook(node.parent, testingFrameworkSetupHooksToFilter);
30+
}
31+
32+
export default ESLintUtils.RuleCreator(getDocsUrl)({
33+
name: RULE_NAME,
34+
meta: {
35+
type: 'problem',
36+
docs: {
37+
description: 'Disallow the use of `render` in setup functions',
38+
category: 'Best Practices',
39+
recommended: false,
40+
},
41+
messages: {
42+
noRenderInSetup:
43+
'Move `render` out of `{{name}}` and into individual tests.',
44+
},
45+
fixable: null,
46+
schema: [
47+
{
48+
type: 'object',
49+
properties: {
50+
renderFunctions: {
51+
type: 'array',
52+
},
53+
allowTestingFrameworkSetupHook: {
54+
enum: TESTING_FRAMEWORK_SETUP_HOOKS,
55+
},
56+
},
57+
anyOf: [
58+
{
59+
required: ['renderFunctions'],
60+
},
61+
{
62+
required: ['allowTestingFrameworkSetupHook'],
63+
},
64+
],
65+
},
66+
],
67+
},
68+
defaultOptions: [
69+
{
70+
renderFunctions: [],
71+
allowTestingFrameworkSetupHook: '',
72+
},
73+
],
74+
75+
create(context, [{ renderFunctions, allowTestingFrameworkSetupHook }]) {
76+
let renderImportedFromTestingLib = false;
77+
let wildcardImportName: string | null = null;
78+
79+
return {
80+
// checks if import has shape:
81+
// import * as dtl from '@testing-library/dom';
82+
'ImportDeclaration[source.value=/testing-library/] ImportNamespaceSpecifier'(
83+
node: TSESTree.ImportNamespaceSpecifier
84+
) {
85+
wildcardImportName = node.local && node.local.name;
86+
},
87+
// checks if `render` is imported from a '@testing-library/foo'
88+
'ImportDeclaration[source.value=/testing-library/]'(
89+
node: TSESTree.ImportDeclaration
90+
) {
91+
renderImportedFromTestingLib = node.specifiers.some(specifier => {
92+
return (
93+
isImportSpecifier(specifier) && specifier.local.name === 'render'
94+
);
95+
});
96+
},
97+
[`VariableDeclarator > CallExpression > Identifier[name="require"]`](
98+
node: TSESTree.Identifier
99+
) {
100+
const {
101+
arguments: callExpressionArgs,
102+
} = node.parent as TSESTree.CallExpression;
103+
const testingLibImport = callExpressionArgs.find(
104+
args =>
105+
isLiteral(args) &&
106+
typeof args.value === 'string' &&
107+
RegExp(/testing-library/, 'g').test(args.value)
108+
);
109+
if (!testingLibImport) {
110+
return;
111+
}
112+
const declaratorNode = node.parent
113+
.parent as TSESTree.VariableDeclarator;
114+
115+
renderImportedFromTestingLib =
116+
isObjectPattern(declaratorNode.id) &&
117+
declaratorNode.id.properties.some(
118+
property =>
119+
isProperty(property) &&
120+
isIdentifier(property.key) &&
121+
property.key.name === 'render'
122+
);
123+
},
124+
CallExpression(node) {
125+
let testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS;
126+
if (allowTestingFrameworkSetupHook.length !== 0) {
127+
testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter(
128+
hook => hook !== allowTestingFrameworkSetupHook
129+
);
130+
}
131+
const beforeHook = findClosestBeforeHook(
132+
node,
133+
testingFrameworkSetupHooksToFilter
134+
);
135+
// if `render` is imported from a @testing-library/foo or
136+
// imported with a wildcard, add `render` to the list of
137+
// disallowed render functions
138+
const disallowedRenderFns =
139+
renderImportedFromTestingLib || wildcardImportName
140+
? ['render', ...renderFunctions]
141+
: renderFunctions;
142+
143+
if (isRenderFunction(node, disallowedRenderFns) && beforeHook) {
144+
context.report({
145+
node,
146+
messageId: 'noRenderInSetup',
147+
data: {
148+
name: beforeHook.name,
149+
},
150+
});
151+
}
152+
},
153+
};
154+
},
155+
});

lib/utils.ts

+3
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ const ASYNC_UTILS = [
6363
'waitForDomChange',
6464
];
6565

66+
const TESTING_FRAMEWORK_SETUP_HOOKS = ['beforeEach', 'beforeAll'];
67+
6668
export {
6769
getDocsUrl,
6870
SYNC_QUERIES_VARIANTS,
@@ -73,5 +75,6 @@ export {
7375
ASYNC_QUERIES_COMBINATIONS,
7476
ALL_QUERIES_COMBINATIONS,
7577
ASYNC_UTILS,
78+
TESTING_FRAMEWORK_SETUP_HOOKS,
7679
LIBRARY_MODULES,
7780
};

0 commit comments

Comments
 (0)