Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(await-async-query): work around false-positives in chained findBy queries #703

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 129 additions & 3 deletions lib/rules/await-async-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
getInnermostReturningFunction,
getVariableReferences,
isPromiseHandled,
isMemberExpression,
isCallExpression,
} from '../node-utils';

export const RULE_NAME = 'await-async-query';
Expand Down Expand Up @@ -39,13 +41,128 @@ export default createTestingLibraryRule<Options, MessageIds>({
defaultOptions: [],

create(context, _, helpers) {
const functionWrappersNames: string[] = [];
const functionWrappersNamesSync: string[] = [];
const functionWrappersNamesAsync: string[] = [];

function detectSyncQueryWrapper(node: TSESTree.Identifier) {
const innerFunction = getInnermostReturningFunction(context, node);
if (innerFunction) {
functionWrappersNamesSync.push(getFunctionName(innerFunction));
}
}

function detectAsyncQueryWrapper(node: TSESTree.Identifier) {
const innerFunction = getInnermostReturningFunction(context, node);
if (innerFunction) {
functionWrappersNames.push(getFunctionName(innerFunction));
functionWrappersNamesAsync.push(getFunctionName(innerFunction));
}
}

function resolveVariable(
node: TSESTree.Node,
scope: ReturnType<typeof context['getScope']> | null
): TSESTree.Node | null {
if (scope == null) {
return null;
}

if (node.type === 'Identifier') {
const variable = scope.variables.find(({ name }) => name === node.name);

// variable not found in this scope, so recursively check parent scope(s) for definition
if (variable == null) {
return resolveVariable(node, scope.upper);
}

if (variable.defs.length === 0) {
return null;
}

const result = variable.defs[variable.defs.length - 1].node;

if (!ASTUtils.isVariableDeclarator(result)) {
return null;
}

return result.init;
}

return node;
}

// true in cases like:
// - getByText('foo').findByType(SomeType)
// - (await findByText('foo')).findByType(SomeType)
// - const variable = await findByText('foo'); variable.findByType(SomeType)
// - function helper() { return screen.getByText('foo'); }; helper().findByType(SomeType)
function hasQueryResultInChain(node: TSESTree.Node): boolean {
if (ASTUtils.isIdentifier(node)) {
return false;
}

if (ASTUtils.isAwaitExpression(node)) {
// great, we have an inline await, so let's check if it's a query
const identifierNode = getDeepestIdentifierNode(node);

if (!identifierNode) {
return false;
}

if (
helpers.isAsyncQuery(identifierNode) &&
isPromiseHandled(identifierNode)
) {
return true;
}

if (
functionWrappersNamesAsync.includes(identifierNode.name) &&
isPromiseHandled(identifierNode)
) {
return true;
}

return false;
}

if (isMemberExpression(node)) {
// check inline sync query (e.g. foo.getByText(...) checks `getByText`)
if (
ASTUtils.isIdentifier(node.property) &&
helpers.isSyncQuery(node.property)
) {
return true;
}

// check sync query reference (e.g. foo.getByText(...) checks `foo` is defined elsewhere)
if (ASTUtils.isIdentifier(node.object)) {
const definition = resolveVariable(node.object, context.getScope());

if (definition == null) {
return false;
}

return hasQueryResultInChain(definition);
}

// check sync query reference (e.g. foo().getByText(...) checks `foo` is defined elsewhere)
if (isCallExpression(node.object)) {
if (
ASTUtils.isIdentifier(node.object.callee) &&
functionWrappersNamesSync.includes(node.object.callee.name)
) {
return true;
}
}

return hasQueryResultInChain(node.object);
}

if (isCallExpression(node)) {
return hasQueryResultInChain(node.callee);
}

return false;
}

return {
Expand All @@ -56,6 +173,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
return;
}

if (helpers.isSyncQuery(identifierNode)) {
detectSyncQueryWrapper(identifierNode);
}

if (helpers.isAsyncQuery(identifierNode)) {
// detect async query used within wrapper function for later analysis
detectAsyncQueryWrapper(identifierNode);
Expand All @@ -69,6 +190,11 @@ export default createTestingLibraryRule<Options, MessageIds>({
return;
}

// check chained usage for an instance of sync query, which means this might be a false positive from react-test-renderer
if (hasQueryResultInChain(closestCallExpressionNode)) {
return;
}

const references = getVariableReferences(
context,
closestCallExpressionNode.parent
Expand Down Expand Up @@ -104,7 +230,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
}
}
} else if (
functionWrappersNames.includes(identifierNode.name) &&
functionWrappersNamesAsync.includes(identifierNode.name) &&
!isPromiseHandled(identifierNode)
) {
// check async queries used within a wrapper previously detected
Expand Down
26 changes: 26 additions & 0 deletions tests/lib/rules/await-async-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,32 @@ ruleTester.run(RULE_NAME, rule, {
// valid async query usage without any function defined
// so there is no innermost function scope found
`const element = await findByRole('button')`,

// react-test-renderer provided `findBy*` queries should be ignored
// https://github.com/testing-library/eslint-plugin-testing-library/issues/673
{
code: `// issue #673
test('this is a valid case', async () => {
const screen = renderContainer()

const syncCall = screen.getByRole('button')
const asyncCall = await screen.findByRole('button')

const syncCall2 = () => { return screen.getByRole('button') }
const asyncCall2 = async () => screen.findByRole('button')

const thing1 = await screen.findByText('foo')
const thing2 = screen.getByText('foo').findByProps({ testID: 'bar' })
const thing3 = (await screen.thing.findByText('foo')).findByProps({ testID: 'bar' })
const thing3a = syncCall.findByProps({ testID: 'bar' })
const thing3b = asyncCall.findByProps({ testID: 'bar' })
const thing3a2 = syncCall2().findByProps({ testID: 'bar' })
const thing3b2 = (await asyncCall2()).findByProps({ testID: 'bar' })
const thing4 = screen.getAllByText('foo')[0].findByProps({ testID: 'bar' })
const thing5 = screen.getAllByText('foo').filter(value => true)[0].findByProps({ testID: 'bar' })
})
`,
},
],

invalid: [
Expand Down