Skip to content

Commit

Permalink
[New] no-cycle: add ignoreExternal option
Browse files Browse the repository at this point in the history
Fixes #1647
  • Loading branch information
sveyret authored and ljharb committed Mar 11, 2020
1 parent 055389d commit 73211e8
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

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

### Fixed
- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])
Expand Down Expand Up @@ -679,6 +680,7 @@ for info on changes for earlier releases.
[#1719]: https://github.com/benmosher/eslint-plugin-import/issues/1719
[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702
[#1690]: https://github.com/benmosher/eslint-plugin-import/pull/1690
[#1681]: https://github.com/benmosher/eslint-plugin-import/pull/1681
[#1676]: https://github.com/benmosher/eslint-plugin-import/pull/1676
[#1666]: https://github.com/benmosher/eslint-plugin-import/pull/1666
[#1664]: https://github.com/benmosher/eslint-plugin-import/pull/1664
Expand Down
23 changes: 23 additions & 0 deletions docs/rules/no-cycle.md
Expand Up @@ -55,6 +55,26 @@ import { b } from './dep-b.js' // not reported as the cycle is at depth 2
This is not necessarily recommended, but available as a cost/benefit tradeoff mechanism
for reducing total project lint time, if needed.

#### `ignoreExternal`

An `ignoreExternal` option is available to prevent the cycle detection to expand to external modules:

```js
/*eslint import/no-cycle: [2, { ignoreExternal: true }]*/

// dep-a.js
import 'module-b/dep-b.js'

export function a() { /* ... */ }
```

```js
// node_modules/module-b/dep-b.js
import { a } from './dep-a.js' // not reported as this module is external
```

Its value is `false` by default, but can be set to `true` for reducing total project lint time, if needed.

## When Not To Use It

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

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

[`no-self-import`]: ./no-self-import.md

[`import/external-module-folders`]: ../../README.md#importexternal-module-folders
12 changes: 12 additions & 0 deletions src/rules/no-cycle.js
Expand Up @@ -4,6 +4,7 @@
*/

import Exports from '../ExportMap'
import { isExternalModule } from '../core/importType'
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor'
import docsUrl from '../docsUrl'

Expand All @@ -18,6 +19,11 @@ module.exports = {
type: 'integer',
minimum: 1,
},
ignoreExternal: {
description: 'ignore external modules',
type: 'boolean',
default: false,
},
})],
},

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

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

function checkSourceValue(sourceNode, importer) {
if (ignoreModule(sourceNode.value)) {
return // ignore external modules
}

const imported = Exports.get(sourceNode.value, context)

if (importer.importKind === 'type') {
Expand All @@ -54,6 +65,7 @@ module.exports = {
for (let [path, { getter, source }] of m.imports) {
if (path === myPath) return true
if (traversed.has(path)) continue
if (ignoreModule(source.value)) continue
if (route.length + 1 < maxDepth) {
untraversed.push({
mget: getter,
Expand Down
2 changes: 2 additions & 0 deletions tests/files/cycles/external-depth-two.js
@@ -0,0 +1,2 @@
import { foo } from "cycles/external/depth-one"
export { foo }
2 changes: 2 additions & 0 deletions tests/files/cycles/external/depth-one.js
@@ -0,0 +1,2 @@
import foo from "../depth-zero"
export { foo }
32 changes: 32 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -40,6 +40,22 @@ ruleTester.run('no-cycle', rule, {
code: 'import { foo, bar } from "./depth-two"',
options: [{ maxDepth: 1 }],
}),
test({
code: 'import { foo } from "cycles/external/depth-one"',
options: [{ ignoreExternal: true }],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
},
}),
test({
code: 'import { foo } from "./external-depth-two"',
options: [{ ignoreExternal: true }],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
},
}),
test({
code: 'import("./depth-two").then(function({ foo }){})',
options: [{ maxDepth: 1 }],
Expand All @@ -63,6 +79,22 @@ ruleTester.run('no-cycle', rule, {
code: 'import { foo } from "./depth-one"',
errors: [error(`Dependency cycle detected.`)],
}),
test({
code: 'import { foo } from "cycles/external/depth-one"',
errors: [error(`Dependency cycle detected.`)],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
},
}),
test({
code: 'import { foo } from "./external-depth-two"',
errors: [error(`Dependency cycle via cycles/external/depth-one:1`)],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
},
}),
test({
code: 'import { foo } from "./depth-one"',
options: [{ maxDepth: 1 }],
Expand Down

0 comments on commit 73211e8

Please sign in to comment.