Skip to content

Commit 5f02f6f

Browse files
sveyretljharb
authored andcommitted
[New] no-cycle: add ignoreExternal option
Fixes #1647
1 parent 055389d commit 5f02f6f

File tree

6 files changed

+73
-0
lines changed

6 files changed

+73
-0
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
88

99
### Added
1010
- [`import/default`]: support default export in TSExportAssignment ([#1528], thanks [@joaovieira])
11+
- [`no-cycle`]: add `ignoreExternal` option ([#1681], thanks [@sveyret])
1112

1213
### Fixed
1314
- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])
@@ -679,6 +680,7 @@ for info on changes for earlier releases.
679680
[#1719]: https://github.com/benmosher/eslint-plugin-import/issues/1719
680681
[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702
681682
[#1690]: https://github.com/benmosher/eslint-plugin-import/pull/1690
683+
[#1681]: https://github.com/benmosher/eslint-plugin-import/pull/1681
682684
[#1676]: https://github.com/benmosher/eslint-plugin-import/pull/1676
683685
[#1666]: https://github.com/benmosher/eslint-plugin-import/pull/1666
684686
[#1664]: https://github.com/benmosher/eslint-plugin-import/pull/1664

docs/rules/no-cycle.md

+23
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,26 @@ import { b } from './dep-b.js' // not reported as the cycle is at depth 2
5555
This is not necessarily recommended, but available as a cost/benefit tradeoff mechanism
5656
for reducing total project lint time, if needed.
5757

58+
#### `ignoreExternal`
59+
60+
An `ignoreExternal` option is available to prevent the cycle detection to expand to external modules:
61+
62+
```js
63+
/*eslint import/no-cycle: [2, { ignoreExternal: true }]*/
64+
65+
// dep-a.js
66+
import 'module-b/dep-b.js'
67+
68+
export function a() { /* ... */ }
69+
```
70+
71+
```js
72+
// node_modules/module-b/dep-b.js
73+
import { a } from './dep-a.js' // not reported as this module is external
74+
```
75+
76+
Its value is `false` by default, but can be set to `true` for reducing total project lint time, if needed.
77+
5878
## When Not To Use It
5979

6080
This rule is comparatively computationally expensive. If you are pressed for lint
@@ -65,5 +85,8 @@ this rule enabled.
6585

6686
- [Original inspiring issue](https://github.com/benmosher/eslint-plugin-import/issues/941)
6787
- Rule to detect that module imports itself: [`no-self-import`]
88+
- [`import/external-module-folders`] setting
6889

6990
[`no-self-import`]: ./no-self-import.md
91+
92+
[`import/external-module-folders`]: ../../README.md#importexternal-module-folders

src/rules/no-cycle.js

+12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import Exports from '../ExportMap'
7+
import { isExternalModule } from '../core/importType'
78
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor'
89
import docsUrl from '../docsUrl'
910

@@ -18,6 +19,11 @@ module.exports = {
1819
type: 'integer',
1920
minimum: 1,
2021
},
22+
ignoreExternal: {
23+
description: 'ignore external modules',
24+
type: 'boolean',
25+
default: false,
26+
},
2127
})],
2228
},
2329

@@ -27,8 +33,13 @@ module.exports = {
2733

2834
const options = context.options[0] || {}
2935
const maxDepth = options.maxDepth || Infinity
36+
const ignoreModule = (name) => options.ignoreExternal ? isExternalModule(name) : false
3037

3138
function checkSourceValue(sourceNode, importer) {
39+
if (ignoreModule(sourceNode.value)) {
40+
return // ignore external modules
41+
}
42+
3243
const imported = Exports.get(sourceNode.value, context)
3344

3445
if (importer.importKind === 'type') {
@@ -54,6 +65,7 @@ module.exports = {
5465
for (let [path, { getter, source }] of m.imports) {
5566
if (path === myPath) return true
5667
if (traversed.has(path)) continue
68+
if (ignoreModule(source.value)) continue
5769
if (route.length + 1 < maxDepth) {
5870
untraversed.push({
5971
mget: getter,
+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { foo } from "cycles/external/depth-one"
2+
export { foo }
+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import foo from "../depth-zero"
2+
export { foo }

tests/src/rules/no-cycle.js

+32
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,22 @@ ruleTester.run('no-cycle', rule, {
4040
code: 'import { foo, bar } from "./depth-two"',
4141
options: [{ maxDepth: 1 }],
4242
}),
43+
test({
44+
code: 'import { foo } from "cycles/external/depth-one"',
45+
options: [{ ignoreExternal: true }],
46+
settings: {
47+
'import/resolver': 'webpack',
48+
'import/external-module-folders': ['external'],
49+
},
50+
}),
51+
test({
52+
code: 'import { foo } from "./external-depth-two"',
53+
options: [{ ignoreExternal: true }],
54+
settings: {
55+
'import/resolver': 'webpack',
56+
'import/external-module-folders': ['external'],
57+
},
58+
}),
4359
test({
4460
code: 'import("./depth-two").then(function({ foo }){})',
4561
options: [{ maxDepth: 1 }],
@@ -63,6 +79,22 @@ ruleTester.run('no-cycle', rule, {
6379
code: 'import { foo } from "./depth-one"',
6480
errors: [error(`Dependency cycle detected.`)],
6581
}),
82+
test({
83+
code: 'import { foo } from "cycles/external/depth-one"',
84+
errors: [error(`Dependency cycle detected.`)],
85+
settings: {
86+
'import/resolver': 'webpack',
87+
'import/external-module-folders': ['external'],
88+
},
89+
}),
90+
test({
91+
code: 'import { foo } from "./external-depth-two"',
92+
errors: [error(`Dependency cycle via cycles/external/depth-one:1`)],
93+
settings: {
94+
'import/resolver': 'webpack',
95+
'import/external-module-folders': ['external'],
96+
},
97+
}),
6698
test({
6799
code: 'import { foo } from "./depth-one"',
68100
options: [{ maxDepth: 1 }],

0 commit comments

Comments
 (0)