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.

Sign up for GitHub

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

Jump to bottom

feat(await-async-events): instance of userEvent is recognized as async #830

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+236 −36
Open

feat(await-async-events): instance of userEvent is recognized as async #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
83 changes: 49 additions & 34 deletions lib/create-testing-library-rule/detect-testing-library-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ type IsAsyncUtilFn = (
validNames?: readonly (typeof ASYNC_UTILS)[number][]
) => boolean;
type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean;
type IsUserEventMethodFn = (node: TSESTree.Identifier) => boolean;
type IsUserEventMethodFn = (
node: TSESTree.Identifier,
userEventSession?: string
) => boolean;
type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean;
type IsCreateEventUtil = (
node: TSESTree.CallExpression | TSESTree.Identifier
Expand All @@ -97,6 +100,9 @@ type FindImportedTestingLibraryUtilSpecifierFn = (
type IsNodeComingFromTestingLibraryFn = (
node: TSESTree.Identifier | TSESTree.MemberExpression
) => boolean;
type getUserEventImportIdentifierFn = (
node: ImportModuleNode | null
) => TSESTree.Identifier | null;

export interface DetectionHelpers {
getTestingLibraryImportNode: GetTestingLibraryImportNodeFn;
Expand Down Expand Up @@ -130,6 +136,7 @@ export interface DetectionHelpers {
canReportErrors: CanReportErrorsFn;
findImportedTestingLibraryUtilSpecifier: FindImportedTestingLibraryUtilSpecifierFn;
isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn;
getUserEventImportIdentifier: getUserEventImportIdentifierFn;
}

const USER_EVENT_PACKAGE = '@testing-library/user-event';
Expand Down Expand Up @@ -326,6 +333,35 @@ export function detectTestingLibraryUtils<
return getImportModuleName(importedCustomModuleNode);
};

const getUserEventImportIdentifier = (node: ImportModuleNode | null) => {
if (!node) {
return null;
}

if (isImportDeclaration(node)) {
Comment on lines +336 to +341
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should always receive a node. If the node is not available, then avoid calling this function in the corresponding code.

Suggested change
const getUserEventImportIdentifier = (node: ImportModuleNode | null) => {
if (!node) {
return null;
}
if (isImportDeclaration(node)) {
const getUserEventImportIdentifier = (node: ImportModuleNode) => {
if (isImportDeclaration(node)) {

const userEventIdentifier = node.specifiers.find((specifier) =>
isImportDefaultSpecifier(specifier)
);

if (userEventIdentifier) {
return userEventIdentifier.local;
}
} else {
if (!ASTUtils.isVariableDeclarator(node.parent)) {
return null;
}

const requireNode = node.parent;
if (!ASTUtils.isIdentifier(requireNode.id)) {
return null;
}

return requireNode.id;
}

return null;
};

/**
* Determines whether Testing Library utils are imported or not for
* current file being analyzed.
Expand Down Expand Up @@ -557,7 +593,10 @@ export function detectTestingLibraryUtils<
return regularCall || wildcardCall || wildcardCallWithCallExpression;
};

const isUserEventMethod: IsUserEventMethodFn = (node) => {
const isUserEventMethod: IsUserEventMethodFn = (
node,
userEventInstance
) => {
const userEvent = findImportedUserEventSpecifier();
let userEventName: string | undefined;

Expand All @@ -567,7 +606,7 @@ export function detectTestingLibraryUtils<
userEventName = USER_EVENT_NAME;
}

if (!userEventName) {
if (!userEventName && !userEventInstance) {
return false;
}

Expand All @@ -591,8 +630,11 @@ export function detectTestingLibraryUtils<

// check userEvent.click() usage
return (
ASTUtils.isIdentifier(parentMemberExpression.object) &&
parentMemberExpression.object.name === userEventName
(ASTUtils.isIdentifier(parentMemberExpression.object) &&
parentMemberExpression.object.name === userEventName) ||
// check userEventInstance.click() usage
(ASTUtils.isIdentifier(parentMemberExpression.object) &&
parentMemberExpression.object.name === userEventInstance)
);
};

Expand Down Expand Up @@ -853,35 +895,7 @@ export function detectTestingLibraryUtils<

const findImportedUserEventSpecifier: () => TSESTree.Identifier | null =
() => {
if (!importedUserEventLibraryNode) {
return null;
}

if (isImportDeclaration(importedUserEventLibraryNode)) {
const userEventIdentifier =
importedUserEventLibraryNode.specifiers.find((specifier) =>
isImportDefaultSpecifier(specifier)
);

if (userEventIdentifier) {
return userEventIdentifier.local;
}
} else {
if (
!ASTUtils.isVariableDeclarator(importedUserEventLibraryNode.parent)
) {
return null;
}

const requireNode = importedUserEventLibraryNode.parent;
if (!ASTUtils.isIdentifier(requireNode.id)) {
return null;
}

return requireNode.id;
}

return null;
return getUserEventImportIdentifier(importedUserEventLibraryNode);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you should call getUserEventImportIdentifier only if importedUserEventLibraryNode exists.

};

const getTestingLibraryImportedUtilSpecifier = (
Expand Down Expand Up @@ -997,6 +1011,7 @@ export function detectTestingLibraryUtils<
canReportErrors,
findImportedTestingLibraryUtilSpecifier,
isNodeComingFromTestingLibrary,
getUserEventImportIdentifier,
};

// Instructions for Testing Library detection.
Expand Down
34 changes: 34 additions & 0 deletions lib/node-utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,3 +679,37 @@ export function findImportSpecifier(
return (property as TSESTree.Property).key as TSESTree.Identifier;
}
}

/**
* Finds if the userEvent is used as an instance
*/

export function getUserEventInstance(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should belong to the detect-testing-library-utils too.

context: TSESLint.RuleContext<string, unknown[]>,
userEventImport: TSESTree.Identifier | null
): string | undefined {
const { tokensAndComments } = context.getSourceCode();
if (!userEventImport) {
return undefined;
}
/**
* Check for the following pattern:
* userEvent.setup(
* For a line like this:
* const user = userEvent.setup();
* function will return 'user'
*/
for (const [index, token] of tokensAndComments.entries()) {
if (
token.type === 'Identifier' &&
token.value === userEventImport.name &&
tokensAndComments[index + 1].value === '.' &&
tokensAndComments[index + 2].value === 'setup' &&
tokensAndComments[index + 3].value === '(' &&
tokensAndComments[index - 1].value === '='
) {
return tokensAndComments[index - 2].value;
Comment on lines +703 to +711
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit brittle and hardcoded… I'm trying to think a better way to implement this in a more generic way with AST selectors.

}
}
return undefined;
}
14 changes: 13 additions & 1 deletion lib/rules/await-async-events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
findClosestFunctionExpressionNode,
getFunctionName,
getInnermostReturningFunction,
getUserEventInstance,
getVariableReferences,
isMemberExpression,
isPromiseHandled,
Expand Down Expand Up @@ -118,9 +119,20 @@ export default createTestingLibraryRule<Options, MessageIds>({

return {
'CallExpression Identifier'(node: TSESTree.Identifier) {
const importedUserEventLibraryNode =
helpers.getTestingLibraryImportNode();
const userEventImport = helpers.getUserEventImportIdentifier(
importedUserEventLibraryNode
);
// Check if userEvent is used as an instance, like const user = userEvent.setup()
const userEventInstance = getUserEventInstance(
context,
userEventImport
);
if (
(isFireEventEnabled && helpers.isFireEventMethod(node)) ||
(isUserEventEnabled && helpers.isUserEventMethod(node))
(isUserEventEnabled &&
helpers.isUserEventMethod(node, userEventInstance))
) {
detectEventMethodWrapper(node);

Expand Down
141 changes: 140 additions & 1 deletion tests/lib/rules/await-async-events.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const USER_EVENT_ASYNC_FUNCTIONS = [
'upload',
] as const;
const FIRE_EVENT_ASYNC_FRAMEWORKS = [
'@testing-library/vue',
// '@testing-library/vue',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be commented.

'@marko/testing-library',
] as const;
const USER_EVENT_ASYNC_FRAMEWORKS = ['@testing-library/user-event'] as const;
Expand Down Expand Up @@ -374,6 +374,27 @@ ruleTester.run(RULE_NAME, rule, {
`,
options: [{ eventModule: ['userEvent', 'fireEvent'] }] as Options,
},
{
code: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('userEvent as instance', async () => {
const user = userEvent.setup()
await user.click(getByLabelText('username'))
})
`,
options: [{ eventModule: ['userEvent'] }] as Options,
},
{
code: `
import u from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('userEvent as named import', async () => {
const user = u.setup()
await user.click(getByLabelText('username'))
await u.click(getByLabelText('username'))
})
`,
options: [{ eventModule: ['userEvent'] }] as Options,
},
]),
],

Expand Down Expand Up @@ -960,6 +981,70 @@ ruleTester.run(RULE_NAME, rule, {
}

triggerEvent()
`,
} as const)
),
...USER_EVENT_ASYNC_FUNCTIONS.map(
(eventMethod) =>
({
code: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('instance of userEvent is recognized as async event', async function() {
const user = userEvent.setup()
user.${eventMethod}(getByLabelText('username'))
})
`,
errors: [
{
line: 5,
column: 5,
messageId: 'awaitAsyncEvent',
data: { name: eventMethod },
},
],
options: [{ eventModule: 'userEvent' }],
output: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('instance of userEvent is recognized as async event', async function() {
const user = userEvent.setup()
await user.${eventMethod}(getByLabelText('username'))
})
`,
} as const)
),
...USER_EVENT_ASYNC_FUNCTIONS.map(
(eventMethod) =>
({
code: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('instance of userEvent is recognized as async event along with static userEvent', async function() {
const user = userEvent.setup()
user.${eventMethod}(getByLabelText('username'))
userEvent.${eventMethod}(getByLabelText('username'))
})
`,
errors: [
{
line: 5,
column: 5,
messageId: 'awaitAsyncEvent',
data: { name: eventMethod },
},
{
line: 6,
column: 5,
messageId: 'awaitAsyncEvent',
data: { name: eventMethod },
},
],
options: [{ eventModule: 'userEvent' }],
output: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('instance of userEvent is recognized as async event along with static userEvent', async function() {
const user = userEvent.setup()
await user.${eventMethod}(getByLabelText('username'))
await userEvent.${eventMethod}(getByLabelText('username'))
})
`,
} as const)
),
Expand Down Expand Up @@ -1021,6 +1106,60 @@ ruleTester.run(RULE_NAME, rule, {
fireEvent.click(getByLabelText('username'))
await userEvent.click(getByLabelText('username'))
})
`,
},
{
code: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
let user;
beforeEach(() => {
user = userEvent.setup()
})
test('instance of userEvent is recognized as async event when instance is initialized in beforeEach', async function() {
user.click(getByLabelText('username'))
})
`,
errors: [
{
line: 8,
column: 5,
messageId: 'awaitAsyncEvent',
data: { name: 'click' },
},
],
output: `
import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
let user;
beforeEach(() => {
user = userEvent.setup()
})
test('instance of userEvent is recognized as async event when instance is initialized in beforeEach', async function() {
await user.click(getByLabelText('username'))
})
`,
},
{
code: `
import u from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('userEvent as named import', async function() {
const user = u.setup()
user.click(getByLabelText('username'))
})
`,
errors: [
{
line: 5,
column: 5,
messageId: 'awaitAsyncEvent',
data: { name: 'click' },
},
],
output: `
import u from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}'
test('userEvent as named import', async function() {
const user = u.setup()
await user.click(getByLabelText('username'))
})
`,
},
],
Expand Down
Loading