Skip to content

Commit

Permalink
fix(no-dom-imports): false negatives with several testing library imp…
Browse files Browse the repository at this point in the history
…orts (#657)
  • Loading branch information
sjarva committed Oct 2, 2022
1 parent e262701 commit 0ae1f25
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 25 deletions.
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' }],
},
],
});

0 comments on commit 0ae1f25

Please sign in to comment.