Skip to content

Commit b92132d

Browse files
authored
feat(prefer-screen-queries): detect render in wrappers (#388)
* refactor: simplify rule implementation * fix(prefer-screen-queries): detect render wrappers properly * test(prefer-screen-queries): add another test case * refactor: remove todo * refactor: simplify nested if statements * refactor: remove commented out code Closes #367
1 parent eb85374 commit b92132d

File tree

2 files changed

+131
-31
lines changed

2 files changed

+131
-31
lines changed

lib/rules/prefer-screen-queries.ts

+53-28
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
22

33
import { createTestingLibraryRule } from '../create-testing-library-rule';
44
import {
5+
getDeepestIdentifierNode,
6+
getFunctionName,
7+
getInnermostReturningFunction,
58
isCallExpression,
69
isMemberExpression,
710
isObjectExpression,
@@ -54,6 +57,22 @@ export default createTestingLibraryRule<Options, MessageIds>({
5457
defaultOptions: [],
5558

5659
create(context, _, helpers) {
60+
const renderWrapperNames: string[] = [];
61+
62+
function detectRenderWrapper(node: TSESTree.Identifier): void {
63+
const innerFunction = getInnermostReturningFunction(context, node);
64+
65+
if (innerFunction) {
66+
renderWrapperNames.push(getFunctionName(innerFunction));
67+
}
68+
}
69+
70+
function isReportableRender(node: TSESTree.Identifier): boolean {
71+
return (
72+
helpers.isRenderUtil(node) || renderWrapperNames.includes(node.name)
73+
);
74+
}
75+
5776
function reportInvalidUsage(node: TSESTree.Identifier) {
5877
context.report({
5978
node,
@@ -78,6 +97,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
7897
}
7998
}
8099

100+
function isIdentifierAllowed(name: string) {
101+
return ['screen', ...withinDeclaredVariables].includes(name);
102+
}
103+
81104
// keep here those queries which are safe and shouldn't be reported
82105
// (from within, from render + container/base element, not related to TL, etc)
83106
const safeDestructuredQueries: string[] = [];
@@ -93,7 +116,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
93116
return;
94117
}
95118

96-
const isComingFromValidRender = helpers.isRenderUtil(node.init.callee);
119+
const isComingFromValidRender = isReportableRender(node.init.callee);
97120

98121
if (!isComingFromValidRender) {
99122
// save the destructured query methods as safe since they are coming
@@ -113,52 +136,54 @@ export default createTestingLibraryRule<Options, MessageIds>({
113136
// save the destructured query methods as safe since they are coming
114137
// from within or render + base/container options
115138
saveSafeDestructuredQueries(node);
116-
return;
117-
}
118-
119-
if (ASTUtils.isIdentifier(node.id)) {
139+
} else if (ASTUtils.isIdentifier(node.id)) {
120140
withinDeclaredVariables.push(node.id.name);
121141
}
122142
},
123-
'CallExpression > Identifier'(node: TSESTree.Identifier) {
124-
if (!helpers.isBuiltInQuery(node)) {
143+
CallExpression(node) {
144+
const identifierNode = getDeepestIdentifierNode(node);
145+
146+
if (!identifierNode) {
125147
return;
126148
}
127149

128-
if (
129-
!safeDestructuredQueries.some((queryName) => queryName === node.name)
130-
) {
131-
reportInvalidUsage(node);
150+
if (helpers.isRenderUtil(identifierNode)) {
151+
detectRenderWrapper(identifierNode);
132152
}
133-
},
134-
'MemberExpression > Identifier'(node: TSESTree.Identifier) {
135-
function isIdentifierAllowed(name: string) {
136-
return ['screen', ...withinDeclaredVariables].includes(name);
153+
154+
if (!helpers.isBuiltInQuery(identifierNode)) {
155+
return;
137156
}
138157

139-
if (!helpers.isBuiltInQuery(node)) {
158+
if (!isMemberExpression(identifierNode.parent)) {
159+
const isSafeDestructuredQuery = safeDestructuredQueries.some(
160+
(queryName) => queryName === identifierNode.name
161+
);
162+
if (isSafeDestructuredQuery) {
163+
return;
164+
}
165+
166+
reportInvalidUsage(identifierNode);
140167
return;
141168
}
142169

170+
const memberExpressionNode = identifierNode.parent;
143171
if (
144-
ASTUtils.isIdentifier(node) &&
145-
isMemberExpression(node.parent) &&
146-
isCallExpression(node.parent.object) &&
147-
ASTUtils.isIdentifier(node.parent.object.callee) &&
148-
node.parent.object.callee.name !== 'within' &&
149-
helpers.isRenderUtil(node.parent.object.callee) &&
150-
!usesContainerOrBaseElement(node.parent.object)
172+
isCallExpression(memberExpressionNode.object) &&
173+
ASTUtils.isIdentifier(memberExpressionNode.object.callee) &&
174+
memberExpressionNode.object.callee.name !== 'within' &&
175+
isReportableRender(memberExpressionNode.object.callee) &&
176+
!usesContainerOrBaseElement(memberExpressionNode.object)
151177
) {
152-
reportInvalidUsage(node);
178+
reportInvalidUsage(identifierNode);
153179
return;
154180
}
155181

156182
if (
157-
isMemberExpression(node.parent) &&
158-
ASTUtils.isIdentifier(node.parent.object) &&
159-
!isIdentifierAllowed(node.parent.object.name)
183+
ASTUtils.isIdentifier(memberExpressionNode.object) &&
184+
!isIdentifierAllowed(memberExpressionNode.object.name)
160185
) {
161-
reportInvalidUsage(node);
186+
reportInvalidUsage(identifierNode);
162187
}
163188
},
164189
};

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

+78-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ ruleTester.run(RULE_NAME, rule, {
4848
(query) => `
4949
import { render } from '@testing-library/react'
5050
import { ${query} } from 'custom-queries'
51-
51+
5252
test("imported custom queries, since they can't be used through screen", () => {
5353
render(foo)
5454
${query}('bar')
@@ -58,7 +58,7 @@ ruleTester.run(RULE_NAME, rule, {
5858
...CUSTOM_QUERY_COMBINATIONS.map(
5959
(query) => `
6060
import { render } from '@testing-library/react'
61-
61+
6262
test("render-returned custom queries, since they can't be used through screen", () => {
6363
const { ${query} } = render(foo)
6464
${query}('bar')
@@ -71,7 +71,7 @@ ruleTester.run(RULE_NAME, rule, {
7171
},
7272
code: `
7373
import { render } from '@testing-library/react'
74-
74+
7575
test("custom queries + custom-queries setting, since they can't be used through screen", () => {
7676
const { ${query} } = render(foo)
7777
${query}('bar')
@@ -413,5 +413,80 @@ ruleTester.run(RULE_NAME, rule, {
413413
],
414414
} as const)
415415
),
416+
{
417+
code: ` // issue #367 - example A
418+
import { render } from '@testing-library/react';
419+
420+
function setup() {
421+
return render(<div />);
422+
}
423+
424+
it('foo', async () => {
425+
const { getByText } = await setup();
426+
expect(getByText('foo')).toBeInTheDocument();
427+
});
428+
429+
it('bar', () => {
430+
const { getByText } = setup();
431+
expect(getByText('foo')).toBeInTheDocument();
432+
});
433+
`,
434+
errors: [
435+
{
436+
messageId: 'preferScreenQueries',
437+
line: 10,
438+
column: 16,
439+
data: {
440+
name: 'getByText',
441+
},
442+
},
443+
{
444+
messageId: 'preferScreenQueries',
445+
line: 15,
446+
column: 16,
447+
data: {
448+
name: 'getByText',
449+
},
450+
},
451+
],
452+
},
453+
{
454+
code: ` // issue #367 - example B
455+
import { render } from '@testing-library/react';
456+
457+
function setup() {
458+
return render(<div />);
459+
}
460+
461+
it('foo', () => {
462+
const { getByText } = setup();
463+
expect(getByText('foo')).toBeInTheDocument();
464+
});
465+
466+
it('bar', () => {
467+
const results = setup();
468+
const { getByText } = results;
469+
expect(getByText('foo')).toBe('foo');
470+
});
471+
`,
472+
errors: [
473+
{
474+
messageId: 'preferScreenQueries',
475+
line: 10,
476+
column: 16,
477+
data: {
478+
name: 'getByText',
479+
},
480+
},
481+
{
482+
messageId: 'preferScreenQueries',
483+
line: 16,
484+
column: 16,
485+
data: {
486+
name: 'getByText',
487+
},
488+
},
489+
],
490+
},
416491
],
417492
});

0 commit comments

Comments
 (0)