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

feat: add new settings for prefer-user-event #251

Merged
merged 3 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 additions & 0 deletions docs/rules/prefer-user-event.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Examples of **incorrect** code for this rule:
```ts
// a method in fireEvent that has a userEvent equivalent
import { fireEvent } from '@testing-library/dom';
// or const { fireEvent } = require('@testing-library/dom');
fireEvent.click(node);

// using fireEvent with an alias
Expand All @@ -26,21 +27,26 @@ fireEventAliased.click(node);

// using fireEvent after importing the entire library
import * as dom from '@testing-library/dom';
// or const dom = require(@testing-library/dom');
dom.fireEvent.click(node);
```

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

```ts
import userEvent from '@testing-library/user-event';
// or const userEvent = require('@testing-library/user-event');

// any userEvent method
userEvent.click();

// fireEvent method that does not have an alternative in userEvent
import { fireEvent } from '@testing-library/dom';
// or const { fireEvent } = require('@testing-library/dom');
fireEvent.cut(node);

import * as dom from '@testing-library/dom';
// or const dom = require('@testing-library/dom');
dom.fireEvent.cut(node);
```

Expand Down Expand Up @@ -69,6 +75,7 @@ With this configuration example, the following use cases are considered valid
```ts
// using a named import
import { fireEvent } from '@testing-library/dom';
// or const { fireEvent } = require('@testing-library/dom');
fireEvent.click(node);
fireEvent.change(node, { target: { value: 'foo' } });

Expand All @@ -79,6 +86,7 @@ fireEventAliased.change(node, { target: { value: 'foo' } });

// using fireEvent after importing the entire library
import * as dom from '@testing-library/dom';
// or const dom = require('@testing-library/dom');
dom.fireEvent.click(node);
dom.fireEvent.change(node, { target: { value: 'foo' } });
```
Expand Down
55 changes: 53 additions & 2 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
import { getImportModuleName, isLiteral, ImportModuleNode } from './node-utils';
import {
getImportModuleName,
isLiteral,
ImportModuleNode,
isImportDeclaration,
isImportNamespaceSpecifier,
isImportSpecifier,
isIdentifier,
isProperty,
} from './node-utils';

export type TestingLibrarySettings = {
'testing-library/module'?: string;
Expand Down Expand Up @@ -33,6 +42,9 @@ export type DetectionHelpers = {
getIsTestingLibraryImported: () => boolean;
getIsValidFilename: () => boolean;
canReportErrors: () => boolean;
findImportedUtilSpecifier: (
specifierName: string
) => TSESTree.ImportClause | TSESTree.Identifier | undefined;
};

const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';
Expand Down Expand Up @@ -106,7 +118,46 @@ export function detectTestingLibraryUtils<
* Wraps all conditions that must be met to report rules.
*/
canReportErrors() {
return this.getIsTestingLibraryImported() && this.getIsValidFilename();
return (
helpers.getIsTestingLibraryImported() && helpers.getIsValidFilename()
);
},
/**
* Gets a string and verifies if it was imported/required by our custom module node
*/
findImportedUtilSpecifier(specifierName: string) {
const node =
helpers.getCustomModuleImportNode() ??
helpers.getTestingLibraryImportNode();
if (!node) {
return null;
}
if (isImportDeclaration(node)) {
const namedExport = node.specifiers.find(
(n) => isImportSpecifier(n) && n.imported.name === specifierName
);
// it is "import { foo [as alias] } from 'baz'""
if (namedExport) {
return namedExport;
}
// it could be "import * as rtl from 'baz'"
return node.specifiers.find((n) => isImportNamespaceSpecifier(n));
} else {
const requireNode = node.parent as TSESTree.VariableDeclarator;
if (isIdentifier(requireNode.id)) {
// this is const rtl = require('foo')
return requireNode.id;
}
// this should be const { something } = require('foo')
const destructuring = requireNode.id as TSESTree.ObjectPattern;
const property = destructuring.properties.find(
(n) =>
isProperty(n) &&
isIdentifier(n.key) &&
n.key.name === specifierName
);
return (property as TSESTree.Property).key as TSESTree.Identifier;
}
},
};

Expand Down
63 changes: 17 additions & 46 deletions lib/rules/prefer-user-event.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl, hasTestingLibraryImportModule } from '../utils';
import {
isImportSpecifier,
isIdentifier,
isMemberExpression,
} from '../node-utils';
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';
import { isIdentifier, isMemberExpression } from '../node-utils';

export const RULE_NAME = 'prefer-user-event';

Expand Down Expand Up @@ -65,7 +61,7 @@ function buildErrorMessage(fireEventMethod: string) {

const fireEventMappedMethods = Object.keys(MappingToUserEvent);

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'suggestion',
Expand All @@ -90,59 +86,34 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
},
defaultOptions: [{ allowedMethods: [] }],

create(context, [options]) {
create(context, [options], helpers) {
const { allowedMethods } = options;
const sourceCode = context.getSourceCode();
let hasNamedImportedFireEvent = false;
let hasImportedFireEvent = false;
let fireEventAlias: string | undefined;
let wildcardImportName: string | undefined;

return {
// checks if import has shape:
// import { fireEvent } from '@testing-library/dom';
ImportDeclaration(node: TSESTree.ImportDeclaration) {
if (!hasTestingLibraryImportModule(node)) {
return;
}
const fireEventImport = node.specifiers.find(
(node) =>
isImportSpecifier(node) && node.imported.name === 'fireEvent'
);
hasNamedImportedFireEvent = !!fireEventImport;
if (!hasNamedImportedFireEvent) {
return;
}
fireEventAlias = fireEventImport.local.name;
},

// checks if import has shape:
// import * as dom from '@testing-library/dom';
'ImportDeclaration ImportNamespaceSpecifier'(
node: TSESTree.ImportNamespaceSpecifier
) {
const importDeclarationNode = node.parent as TSESTree.ImportDeclaration;
if (!hasTestingLibraryImportModule(importDeclarationNode)) {
return;
}
hasImportedFireEvent = !!node.local.name;
wildcardImportName = node.local.name;
},
['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) {
if (!hasImportedFireEvent && !hasNamedImportedFireEvent) {
const util = helpers.findImportedUtilSpecifier('fireEvent');
const fireEventAliasOrWildcard = isIdentifier(util)
? util?.name
: util?.local.name;

if (!fireEventAliasOrWildcard) {
// testing library was imported, but fireEvent was not imported
return;
}
// check node is fireEvent or it's alias from the named import
const fireEventUsed =
isIdentifier(node.object) && node.object.name === fireEventAlias;
isIdentifier(node.object) &&
node.object.name === fireEventAliasOrWildcard;

const fireEventFromWildcardUsed =
isMemberExpression(node.object) &&
isIdentifier(node.object.object) &&
node.object.object.name === wildcardImportName &&
node.object.object.name === fireEventAliasOrWildcard &&
isIdentifier(node.object.property) &&
node.object.property.name === 'fireEvent';
gndelia marked this conversation as resolved.
Show resolved Hide resolved

if (!fireEventUsed && !fireEventFromWildcardUsed) {
// fireEvent was imported but it was not used
return;
}

Expand Down
109 changes: 109 additions & 0 deletions tests/lib/rules/prefer-user-event.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,67 @@ ruleTester.run(RULE_NAME, rule, {
fireEvent()
`,
})),
{
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { screen } from 'test-utils'
const element = screen.getByText(foo)
`,
},
{
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { render } from 'test-utils'
const utils = render(baz)
const element = utils.getByText(foo)
`,
},
...UserEventMethods.map((userEventMethod) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import userEvent from 'test-utils'
const node = document.createElement(elementType)
userEvent.${userEventMethod}(foo)
`,
})),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { fireEvent } from 'test-utils'
const node = document.createElement(elementType)
fireEvent.${fireEventMethod}(foo)
`,
options: [{ allowedMethods: [fireEventMethod] }],
})),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { fireEvent as fireEventAliased } from 'test-utils'
const node = document.createElement(elementType)
fireEventAliased.${fireEventMethod}(foo)
`,
options: [{ allowedMethods: [fireEventMethod] }],
})),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import * as dom from 'test-utils'
dom.fireEvent.${fireEventMethod}(foo)
`,
options: [{ allowedMethods: [fireEventMethod] }],
})),
],
invalid: [
...createScenarioWithImport<InvalidTestCase<MessageIds, Options>>(
Expand All @@ -130,5 +191,53 @@ ruleTester.run(RULE_NAME, rule, {
errors: [{ messageId: 'preferUserEvent' }],
})
),
...createScenarioWithImport<InvalidTestCase<MessageIds, Options>>(
(libraryModule: string, fireEventMethod: string) => ({
code: `
const { fireEvent } = require('${libraryModule}')
fireEvent.${fireEventMethod}(foo)
`,
errors: [{ messageId: 'preferUserEvent' }],
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to check line and column of the error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added in 06cb9cf

})
),
...createScenarioWithImport<InvalidTestCase<MessageIds, Options>>(
(libraryModule: string, fireEventMethod: string) => ({
code: `
const rtl = require('${libraryModule}')
rtl.fireEvent.${fireEventMethod}(foo)
`,
errors: [{ messageId: 'preferUserEvent' }],
})
),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import * as dom from 'test-utils'
dom.fireEvent.${fireEventMethod}(foo)
`,
errors: [{ messageId: 'preferUserEvent' }],
})),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { fireEvent } from 'test-utils'
fireEvent.${fireEventMethod}(foo)
`,
errors: [{ messageId: 'preferUserEvent' }],
})),
...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({
settings: {
'testing-library/module': 'test-utils',
},
code: `
import { fireEvent as fireEventAliased } from 'test-utils'
fireEventAliased.${fireEventMethod}(foo)
`,
errors: [{ messageId: 'preferUserEvent' }],
})),
],
});