Skip to content

Commit 28238aa

Browse files
authored
fix(prefer-screen-queries): false positives when using within method (#119)
Closes #116
1 parent 1501fa4 commit 28238aa

File tree

3 files changed

+176
-2
lines changed

3 files changed

+176
-2
lines changed

docs/rules/prefer-screen-queries.md

+20
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,37 @@ Examples of **incorrect** code for this rule:
1212
const { getByText } = render(<Component />);
1313
getByText('foo');
1414

15+
// calling a query from a variable returned from a `render` method
1516
const utils = render(<Component />);
1617
utils.getByText('foo');
18+
19+
// using after render
20+
render(<Component />).getByText('foo');
21+
22+
// calling a query from a custom `render` method that returns an array
23+
const [getByText] = myCustomRender(<Component />);
24+
getByText('foo');
1725
```
1826

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

2129
```js
2230
import { screen } from '@testing-library/any-framework';
2331

32+
// calling a query from the `screen` object
2433
render(<Component />);
2534
screen.getByText('foo');
35+
36+
// using after within clause
37+
within(screen.getByTestId('section')).getByText('foo');
38+
39+
// calling a query method returned from a within call
40+
const { getByText } = within(screen.getByText('foo'));
41+
getByText('foo');
42+
43+
// calling a method directly from a variable created by within
44+
const myWithinVariable = within(screen.getByText('foo'));
45+
myWithinVariable.getByText('foo');
2646
```
2747

2848
## Further Reading

lib/rules/prefer-screen-queries.ts

+70-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
22
import { getDocsUrl, ALL_QUERIES_COMBINATIONS } from '../utils';
3+
import {
4+
isMemberExpression,
5+
isObjectPattern,
6+
isCallExpression,
7+
isProperty,
8+
isIdentifier,
9+
} from '../node-utils';
310

411
export const RULE_NAME = 'prefer-screen-queries';
512
export type MessageIds = 'preferScreenQueries';
@@ -36,9 +43,70 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
3643
});
3744
}
3845

46+
const queriesRegex = new RegExp(ALL_QUERIES_COMBINATIONS_REGEXP);
47+
const queriesDestructuredInWithinDeclaration: string[] = [];
48+
// use an array as within might be used more than once in a test
49+
const withinDeclaredVariables : string[] = []
50+
3951
return {
40-
[`CallExpression > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`]: reportInvalidUsage,
41-
[`MemberExpression[object.name!="screen"] > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`]: reportInvalidUsage,
52+
VariableDeclarator(node) {
53+
const isWithinFunction = isCallExpression(node.init) && isIdentifier(node.init.callee) && node.init.callee.name === 'within';
54+
55+
if (!isWithinFunction) {
56+
return
57+
}
58+
59+
if (isObjectPattern(node.id)) {
60+
// save the destructured query methods
61+
const identifiers = node.id.properties
62+
.filter(property => isProperty(property) && isIdentifier(property.key) && queriesRegex.test(property.key.name))
63+
.map((property: TSESTree.Property) => (property.key as TSESTree.Identifier).name);
64+
65+
queriesDestructuredInWithinDeclaration.push(...identifiers);
66+
return
67+
}
68+
69+
if (isIdentifier(node.id)) {
70+
withinDeclaredVariables.push(node.id.name)
71+
}
72+
},
73+
[`CallExpression > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`](
74+
node: TSESTree.Identifier
75+
) {
76+
if (
77+
!queriesDestructuredInWithinDeclaration.some(
78+
queryName => queryName === node.name
79+
)
80+
) {
81+
reportInvalidUsage(node);
82+
}
83+
},
84+
[`MemberExpression > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`](
85+
node: TSESTree.Identifier
86+
) {
87+
88+
function isIdentifierAllowed(name: string) {
89+
return ['screen', ...withinDeclaredVariables].includes(name)
90+
}
91+
92+
if (
93+
isIdentifier(node) &&
94+
isMemberExpression(node.parent) &&
95+
isCallExpression(node.parent.object) &&
96+
isIdentifier(node.parent.object.callee) &&
97+
node.parent.object.callee.name !== 'within'
98+
) {
99+
reportInvalidUsage(node);
100+
return;
101+
}
102+
if (
103+
isMemberExpression(node.parent) &&
104+
isIdentifier(node.parent.object) &&
105+
!isIdentifierAllowed(node.parent.object.name)
106+
) {
107+
reportInvalidUsage(node);
108+
}
109+
},
42110
};
43111
},
44112
});

tests/lib/rules/prefer-screen-queries.test.ts

+86
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,24 @@ ruleTester.run(RULE_NAME, rule, {
1515
{
1616
code: `component.otherFunctionShouldNotThrow()`,
1717
},
18+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
19+
code: `within(component).${queryMethod}()`,
20+
})),
21+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
22+
code: `within(screen.${queryMethod}()).${queryMethod}()`,
23+
})),
24+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
25+
code: `
26+
const { ${queryMethod} } = within(screen.getByText('foo'))
27+
${queryMethod}(baz)
28+
`,
29+
})),
30+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
31+
code: `
32+
const myWithinVariable = within(foo)
33+
myWithinVariable.${queryMethod}('baz')
34+
`,
35+
})),
1836
],
1937

2038
invalid: [
@@ -30,6 +48,18 @@ ruleTester.run(RULE_NAME, rule, {
3048
],
3149
})),
3250

51+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
52+
code: `render().${queryMethod}()`,
53+
errors: [
54+
{
55+
messageId: 'preferScreenQueries',
56+
data: {
57+
name: queryMethod,
58+
},
59+
},
60+
],
61+
})),
62+
3363
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
3464
code: `component.${queryMethod}()`,
3565
errors: [
@@ -41,5 +71,61 @@ ruleTester.run(RULE_NAME, rule, {
4171
},
4272
],
4373
})),
74+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
75+
code: `
76+
const { ${queryMethod} } = render()
77+
${queryMethod}(baz)
78+
`,
79+
errors: [
80+
{
81+
messageId: 'preferScreenQueries',
82+
data: {
83+
name: queryMethod,
84+
},
85+
},
86+
],
87+
})),
88+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
89+
code: `
90+
const myRenderVariable = render()
91+
myRenderVariable.${queryMethod}(baz)
92+
`,
93+
errors: [
94+
{
95+
messageId: 'preferScreenQueries',
96+
data: {
97+
name: queryMethod,
98+
},
99+
},
100+
],
101+
})),
102+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
103+
code: `
104+
const [myVariable] = render()
105+
myVariable.${queryMethod}(baz)
106+
`,
107+
errors: [
108+
{
109+
messageId: 'preferScreenQueries',
110+
data: {
111+
name: queryMethod,
112+
},
113+
},
114+
],
115+
})),
116+
...ALL_QUERIES_COMBINATIONS.map(queryMethod => ({
117+
code: `
118+
const [myVariable] = within()
119+
myVariable.${queryMethod}(baz)
120+
`,
121+
errors: [
122+
{
123+
messageId: 'preferScreenQueries',
124+
data: {
125+
name: queryMethod,
126+
},
127+
},
128+
],
129+
})),
44130
],
45131
});

0 commit comments

Comments
 (0)