Skip to content

fix(no-dom-imports): false negatives with several testing library imports #657

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

Merged
Merged
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
5 changes: 5 additions & 0 deletions docs/rules/no-dom-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ import { fireEvent } from 'dom-testing-library';
import { fireEvent } from '@testing-library/dom';
```

```js
import { render } from '@testing-library/react'; // Okay, no error
import { screen } from '@testing-library/dom'; // Error, unnecessary import from @testing-library/dom
```

```js
const { fireEvent } = require('dom-testing-library');
```
Expand Down
28 changes: 16 additions & 12 deletions lib/create-testing-library-rule/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export type EnhancedRuleCreate<

// Helpers methods
type GetTestingLibraryImportNodeFn = () => ImportModuleNode | null;
type GetTestingLibraryImportNodesFn = () => ImportModuleNode[];
type GetCustomModuleImportNodeFn = () => ImportModuleNode | null;
type GetTestingLibraryImportNameFn = () => string | undefined;
type GetCustomModuleImportNameFn = () => string | undefined;
Expand Down Expand Up @@ -95,6 +96,7 @@ type IsNodeComingFromTestingLibraryFn = (

export interface DetectionHelpers {
getTestingLibraryImportNode: GetTestingLibraryImportNodeFn;
getAllTestingLibraryImportNodes: GetTestingLibraryImportNodesFn;
getCustomModuleImportNode: GetCustomModuleImportNodeFn;
getTestingLibraryImportName: GetTestingLibraryImportNameFn;
getCustomModuleImportName: GetCustomModuleImportNameFn;
Expand Down Expand Up @@ -158,7 +160,7 @@ export function detectTestingLibraryUtils<
context: TestingLibraryContext<TOptions, TMessageIds>,
optionsWithDefault: Readonly<TOptions>
): TSESLint.RuleListener => {
let importedTestingLibraryNode: ImportModuleNode | null = null;
const importedTestingLibraryNodes: ImportModuleNode[] = [];
let importedCustomModuleNode: ImportModuleNode | null = null;
let importedUserEventLibraryNode: ImportModuleNode | null = null;
let importedReactDomTestUtilsNode: ImportModuleNode | null = null;
Expand Down Expand Up @@ -299,15 +301,20 @@ export function detectTestingLibraryUtils<

// Helpers for Testing Library detection.
const getTestingLibraryImportNode: GetTestingLibraryImportNodeFn = () => {
return importedTestingLibraryNode;
return importedTestingLibraryNodes[0];
};

const getAllTestingLibraryImportNodes: GetTestingLibraryImportNodesFn =
() => {
return importedTestingLibraryNodes;
};

const getCustomModuleImportNode: GetCustomModuleImportNodeFn = () => {
return importedCustomModuleNode;
};

const getTestingLibraryImportName: GetTestingLibraryImportNameFn = () => {
return getImportModuleName(importedTestingLibraryNode);
return getImportModuleName(importedTestingLibraryNodes[0]);
};

const getCustomModuleImportName: GetCustomModuleImportNameFn = () => {
Expand All @@ -331,7 +338,7 @@ export function detectTestingLibraryUtils<
isStrict = false
) => {
const isSomeModuleImported =
!!importedTestingLibraryNode || !!importedCustomModuleNode;
importedTestingLibraryNodes.length !== 0 || !!importedCustomModuleNode;

return (
(!isStrict && isAggressiveModuleReportingEnabled()) ||
Expand Down Expand Up @@ -945,6 +952,7 @@ export function detectTestingLibraryUtils<

const helpers: DetectionHelpers = {
getTestingLibraryImportNode,
getAllTestingLibraryImportNodes,
getCustomModuleImportNode,
getTestingLibraryImportName,
getCustomModuleImportName,
Expand Down Expand Up @@ -989,12 +997,9 @@ export function detectTestingLibraryUtils<
return;
}
// check only if testing library import not found yet so we avoid
// to override importedTestingLibraryNode after it's found
if (
!importedTestingLibraryNode &&
/testing-library/g.test(node.source.value)
) {
importedTestingLibraryNode = node;
// to override importedTestingLibraryNodes after it's found
if (/testing-library/g.test(node.source.value)) {
importedTestingLibraryNodes.push(node);
}

// check only if custom module import not found yet so we avoid
Expand Down Expand Up @@ -1035,15 +1040,14 @@ export function detectTestingLibraryUtils<
const { arguments: args } = callExpression;

if (
!importedTestingLibraryNode &&
args.some(
(arg) =>
isLiteral(arg) &&
typeof arg.value === 'string' &&
/testing-library/g.test(arg.value)
)
) {
importedTestingLibraryNode = callExpression;
importedTestingLibraryNodes.push(callExpression);
}

const customModule = getCustomModule();
Expand Down
26 changes: 13 additions & 13 deletions lib/rules/no-dom-import.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { TSESTree } from '@typescript-eslint/utils';

import { createTestingLibraryRule } from '../create-testing-library-rule';
import { isCallExpression } from '../node-utils';
import { isCallExpression, getImportModuleName } from '../node-utils';

export const RULE_NAME = 'no-dom-import';
export type MessageIds = 'noDomImport' | 'noDomImportFramework';
Expand Down Expand Up @@ -84,22 +84,22 @@ export default createTestingLibraryRule<Options, MessageIds>({

return {
'Program:exit'() {
const importName = helpers.getTestingLibraryImportName();
const importNode = helpers.getTestingLibraryImportNode();
let importName: string | undefined;
const allImportNodes = helpers.getAllTestingLibraryImportNodes();

if (!importNode) {
return;
}
allImportNodes.forEach((importNode) => {
importName = getImportModuleName(importNode);

const domModuleName = DOM_TESTING_LIBRARY_MODULES.find(
(module) => module === importName
);
const domModuleName = DOM_TESTING_LIBRARY_MODULES.find(
(module) => module === importName
);

if (!domModuleName) {
return;
}
if (!domModuleName) {
return;
}

report(importNode, domModuleName);
report(importNode, domModuleName);
});
},
};
},
Expand Down
12 changes: 12 additions & 0 deletions tests/lib/rules/no-dom-import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,5 +203,17 @@ ruleTester.run(RULE_NAME, rule, {
code: 'require("@testing-library/dom")',
errors: [{ messageId: 'noDomImport' }],
},
{
code: `
require("@testing-library/dom");
require("@testing-library/react");`,
errors: [{ line: 2, messageId: 'noDomImport' }],
},
{
code: `
import { render } from '@testing-library/react';
import { screen } from '@testing-library/dom';`,
errors: [{ line: 3, messageId: 'noDomImport' }],
},
],
});