Skip to content

Commit 17e3cfe

Browse files
Belco90Belco90
committed
Merge remote-tracking branch 'origin/master' into v4
2 parents 9aa6730 + 993fd8c commit 17e3cfe

File tree

2 files changed

+198
-51
lines changed

2 files changed

+198
-51
lines changed

‎lib/rules/prefer-find-by.ts‎

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { ReportFixFunction, Scope, RuleFix } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
23
import {
34
isIdentifier,
45
isCallExpression,
56
isMemberExpression,
67
isArrowFunctionExpression,
8+
isObjectPattern,
9+
isProperty,
710
} from '../node-utils';
811
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';
912

@@ -34,24 +37,22 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
3437
create(context) {
3538
const sourceCode = context.getSourceCode();
3639

40+
41+
3742
/**
3843
* Reports the invalid usage of wait* plus getBy/QueryBy methods and automatically fixes the scenario
3944
* @param {TSESTree.CallExpression} node - The CallExpresion node that contains the wait* method
4045
* @param {'findBy' | 'findAllBy'} replacementParams.queryVariant - The variant method used to query: findBy/findByAll.
4146
* @param {string} replacementParams.queryMethod - Suffix string to build the query method (the query-part that comes after the "By"): LabelText, Placeholder, Text, Role, Title, etc.
42-
* @param {Array<TSESTree.Expression>} replacementParams.callArguments - Array of argument nodes which contain the parameters of the query inside the wait* method.
43-
* @param {string=} replacementParams.caller - the variable name that targets screen or the value returned from `render` function.
47+
* @param {ReportFixFunction} replacementParams.fix - Function that applies the fix to correct the code
4448
*/
45-
function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, callArguments, caller }: { queryVariant: 'findBy' | 'findAllBy', queryMethod: string, callArguments: TSESTree.Expression[], caller?: string }) {
49+
function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, fix }: { queryVariant: 'findBy' | 'findAllBy', queryMethod: string, fix: ReportFixFunction }) {
4650

4751
context.report({
4852
node,
4953
messageId: "preferFindBy",
5054
data: { queryVariant, queryMethod, fullQuery: sourceCode.getText(node) },
51-
fix(fixer) {
52-
const newCode = `${caller ? `${caller}.` : ''}${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})`
53-
return fixer.replaceText(node, newCode)
54-
}
55+
fix,
5556
});
5657
}
5758

@@ -72,24 +73,60 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
7273
// ensure here it's one of the sync methods that we are calling
7374
if (isMemberExpression(argument.body.callee) && isIdentifier(argument.body.callee.property) && isIdentifier(argument.body.callee.object) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)) {
7475
// shape of () => screen.getByText
75-
const queryMethod = argument.body.callee.property.name
76+
const fullQueryMethod = argument.body.callee.property.name
7677
const caller = argument.body.callee.object.name
77-
78+
const queryVariant = getFindByQueryVariant(fullQueryMethod)
79+
const callArguments = argument.body.arguments
80+
const queryMethod = fullQueryMethod.split('By')[1]
81+
7882
reportInvalidUsage(node, {
79-
queryMethod: queryMethod.split('By')[1],
80-
queryVariant: getFindByQueryVariant(queryMethod),
81-
callArguments: argument.body.arguments,
82-
caller,
83+
queryMethod,
84+
queryVariant,
85+
fix(fixer) {
86+
const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})`
87+
return fixer.replaceText(node, newCode)
88+
}
8389
})
8490
return
8591
}
8692
if (isIdentifier(argument.body.callee) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)) {
8793
// shape of () => getByText
88-
const queryMethod = argument.body.callee.name
94+
const fullQueryMethod = argument.body.callee.name
95+
const queryMethod = fullQueryMethod.split('By')[1]
96+
const queryVariant = getFindByQueryVariant(fullQueryMethod)
97+
const callArguments = argument.body.arguments
98+
8999
reportInvalidUsage(node, {
90-
queryMethod: queryMethod.split('By')[1],
91-
queryVariant: getFindByQueryVariant(queryMethod),
92-
callArguments: argument.body.arguments,
100+
queryMethod,
101+
queryVariant,
102+
fix(fixer) {
103+
const findByMethod = `${queryVariant}${queryMethod}`
104+
const allFixes: RuleFix[] = []
105+
// this updates waitFor with findBy*
106+
const newCode = `${findByMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})`
107+
allFixes.push(fixer.replaceText(node, newCode))
108+
109+
// this adds the findBy* declaration - adding it to the list of destructured variables { findBy* } = render()
110+
const definition = findRenderDefinitionDeclaration(context.getScope(), fullQueryMethod)
111+
// I think it should always find it, otherwise code should not be valid (it'd be using undeclared variables)
112+
if (!definition) {
113+
return allFixes
114+
}
115+
// check the declaration is part of a destructuring
116+
if (isObjectPattern(definition.parent.parent)) {
117+
const allVariableDeclarations = definition.parent.parent
118+
// verify if the findBy* method was already declared
119+
if (allVariableDeclarations.properties.some((p) => isProperty(p) && isIdentifier(p.key) && p.key.name === findByMethod)) {
120+
return allFixes
121+
}
122+
// the last character of a destructuring is always a "}", so we should replace it with the findBy* declaration
123+
const textDestructuring = sourceCode.getText(allVariableDeclarations)
124+
const text = textDestructuring.substring(0, textDestructuring.length - 2) + `, ${findByMethod} }`
125+
allFixes.push(fixer.replaceText(allVariableDeclarations, text))
126+
}
127+
128+
return allFixes
129+
}
93130
})
94131
return
95132
}
@@ -98,6 +135,18 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
98135
}
99136
})
100137

101-
function getFindByQueryVariant(queryMethod: string) {
138+
export function getFindByQueryVariant(queryMethod: string) {
102139
return queryMethod.includes('All') ? 'findAllBy' : 'findBy'
140+
}
141+
142+
function findRenderDefinitionDeclaration(scope: Scope.Scope | null, query: string): TSESTree.Identifier | null {
143+
if (!scope) {
144+
return null
145+
}
146+
let variable = scope.variables.find((v: Scope.Variable) => v.name === query)
147+
if (variable) {
148+
const def = variable.defs.find(({ name }) => name.name === query)
149+
return def.name
150+
}
151+
return findRenderDefinitionDeclaration(scope.upper, query)
103152
}

‎tests/lib/rules/prefer-find-by.test.ts‎

Lines changed: 131 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,34 @@
1-
import { InvalidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
1+
import { InvalidTestCase, ValidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
22
import { createRuleTester } from '../test-utils';
33
import { ASYNC_QUERIES_COMBINATIONS, SYNC_QUERIES_COMBINATIONS } from '../../../lib/utils';
4-
import rule, { WAIT_METHODS, RULE_NAME } from '../../../lib/rules/prefer-find-by';
4+
import rule, { WAIT_METHODS, RULE_NAME, getFindByQueryVariant, MessageIds } from '../../../lib/rules/prefer-find-by';
55

66
const ruleTester = createRuleTester({
77
ecmaFeatures: {
88
jsx: true,
99
},
1010
});
1111

12+
function buildFindByMethod(queryMethod: string) {
13+
return `${getFindByQueryVariant(queryMethod)}${queryMethod.split('By')[1]}`
14+
}
15+
16+
function createScenario<T extends ValidTestCase<[]> | InvalidTestCase<MessageIds, []>>(callback: (waitMethod: string, queryMethod: string) => T) {
17+
return WAIT_METHODS.reduce((acc: T[], waitMethod) =>
18+
acc.concat(
19+
SYNC_QUERIES_COMBINATIONS
20+
.map((queryMethod) => callback(waitMethod, queryMethod))
21+
)
22+
, [])
23+
}
24+
1225
ruleTester.run(RULE_NAME, rule, {
1326
valid: [
1427
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
15-
code: `const submitButton = await ${queryMethod}('foo')`
28+
code: `
29+
const { ${queryMethod} } = setup()
30+
const submitButton = await ${queryMethod}('foo')
31+
`
1632
})),
1733
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
1834
code: `const submitButton = await screen.${queryMethod}('foo')`
@@ -60,35 +76,117 @@ ruleTester.run(RULE_NAME, rule, {
6076
}
6177
],
6278
invalid: [
63-
// using reduce + concat 'cause flatMap is not available in node10.x
64-
...WAIT_METHODS.reduce((acc: InvalidTestCase<'preferFindBy', []>[], waitMethod) => acc
65-
.concat(
66-
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
67-
code: `const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
68-
errors: [{
69-
messageId: 'preferFindBy',
70-
data: {
71-
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
72-
queryMethod: queryMethod.split('By')[1],
73-
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
74-
},
75-
}],
76-
output: `const submitButton = await ${queryMethod.includes('All') ? 'findAllBy': 'findBy'}${queryMethod.split('By')[1]}('foo', { name: 'baz' })`
77-
}))
78-
).concat(
79-
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
80-
code: `const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
81-
errors: [{
82-
messageId: 'preferFindBy',
83-
data: {
84-
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
85-
queryMethod: queryMethod.split('By')[1],
86-
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
87-
}
88-
}],
89-
output: `const submitButton = await screen.${queryMethod.includes('All') ? 'findAllBy': 'findBy'}${queryMethod.split('By')[1]}('foo', { name: 'baz' })`
90-
}))
91-
),
92-
[])
79+
...createScenario((waitMethod: string, queryMethod: string) => ({
80+
code: `
81+
const { ${queryMethod} } = render()
82+
const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))
83+
`,
84+
errors: [{
85+
messageId: 'preferFindBy',
86+
data: {
87+
queryVariant: getFindByQueryVariant(queryMethod),
88+
queryMethod: queryMethod.split('By')[1],
89+
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
90+
},
91+
}],
92+
output: `
93+
const { ${queryMethod}, ${buildFindByMethod(queryMethod)} } = render()
94+
const submitButton = await ${buildFindByMethod(queryMethod)}('foo', { name: 'baz' })
95+
`
96+
})),
97+
...createScenario((waitMethod: string, queryMethod: string) => ({
98+
code: `const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
99+
errors: [{
100+
messageId: 'preferFindBy',
101+
data: {
102+
queryVariant: getFindByQueryVariant(queryMethod),
103+
queryMethod: queryMethod.split('By')[1],
104+
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
105+
}
106+
}],
107+
output: `const submitButton = await screen.${buildFindByMethod(queryMethod)}('foo', { name: 'baz' })`
108+
})),
109+
// // this scenario verifies it works when the render function is defined in another scope
110+
...WAIT_METHODS.map((waitMethod: string) => ({
111+
code: `
112+
const { getByText, queryByLabelText, findAllByRole } = customRender()
113+
it('foo', async () => {
114+
const submitButton = await ${waitMethod}(() => getByText('baz', { name: 'button' }))
115+
})
116+
`,
117+
errors: [{
118+
messageId: 'preferFindBy',
119+
data: {
120+
queryVariant: 'findBy',
121+
queryMethod: 'Text',
122+
fullQuery: `${waitMethod}(() => getByText('baz', { name: 'button' }))`,
123+
}
124+
}],
125+
output: `
126+
const { getByText, queryByLabelText, findAllByRole, findByText } = customRender()
127+
it('foo', async () => {
128+
const submitButton = await findByText('baz', { name: 'button' })
129+
})
130+
`
131+
})),
132+
// // this scenario verifies when findBy* were already defined (because it was used elsewhere)
133+
...WAIT_METHODS.map((waitMethod: string) => ({
134+
code: `
135+
const { getAllByRole, findAllByRole } = customRender()
136+
describe('some scenario', () => {
137+
it('foo', async () => {
138+
const submitButton = await ${waitMethod}(() => getAllByRole('baz', { name: 'button' }))
139+
})
140+
})
141+
`,
142+
errors: [{
143+
messageId: 'preferFindBy',
144+
data: {
145+
queryVariant: 'findAllBy',
146+
queryMethod: 'Role',
147+
fullQuery: `${waitMethod}(() => getAllByRole('baz', { name: 'button' }))`,
148+
}
149+
}],
150+
output: `
151+
const { getAllByRole, findAllByRole } = customRender()
152+
describe('some scenario', () => {
153+
it('foo', async () => {
154+
const submitButton = await findAllByRole('baz', { name: 'button' })
155+
})
156+
})
157+
`
158+
})),
159+
// invalid code, as we need findBy* to be defined somewhere, but required for getting 100% coverage
160+
{
161+
code: `const submitButton = await waitFor(() => getByText('baz', { name: 'button' }))`,
162+
errors: [{
163+
messageId: 'preferFindBy',
164+
data: {
165+
queryVariant: 'findBy',
166+
queryMethod: 'Text',
167+
fullQuery: `waitFor(() => getByText('baz', { name: 'button' }))`
168+
}
169+
}],
170+
output: `const submitButton = await findByText('baz', { name: 'button' })`
171+
},
172+
// this code would be invalid too, as findByRole is not defined anywhere.
173+
{
174+
code: `
175+
const getByRole = render().getByRole
176+
const submitButton = await waitFor(() => getByRole('baz', { name: 'button' }))
177+
`,
178+
errors: [{
179+
messageId: 'preferFindBy',
180+
data: {
181+
queryVariant: 'findBy',
182+
queryMethod: 'Role',
183+
fullQuery: `waitFor(() => getByRole('baz', { name: 'button' }))`
184+
}
185+
}],
186+
output: `
187+
const getByRole = render().getByRole
188+
const submitButton = await findByRole('baz', { name: 'button' })
189+
`
190+
}
93191
],
94192
})

0 commit comments

Comments
 (0)