Skip to content

Commit

Permalink
no-cycle: maxDepth tests + docs
Browse files Browse the repository at this point in the history
  • Loading branch information
benmosher committed Mar 26, 2018
1 parent 6933fa4 commit d81f48a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 9 deletions.
40 changes: 33 additions & 7 deletions docs/rules/no-cycle.md
Expand Up @@ -2,11 +2,8 @@

Ensures that there is no resolvable path back to this module via its dependencies.

By default, this rule only detects cycles for ES6 imports, but see the [`no-unresolved` options](./no-unresolved.md#options) as this rule also supports the same `commonjs` and `amd` flags. However, these flags only impact which import types are _linted_; the
import/export infrastructure only registers `import` statements in dependencies, so
cycles created by `require` within imported modules may not be detected.

This includes cycles of depth 1 (imported module imports me) to `Infinity`.
This includes cycles of depth 1 (imported module imports me) to `Infinity`, if the
[`maxDepth`](#maxdepth) option is not set.

```js
// dep-b.js
Expand All @@ -24,10 +21,39 @@ for that, see [`no-self-import`].

## Rule Details

### Options

By default, this rule only detects cycles for ES6 imports, but see the [`no-unresolved` options](./no-unresolved.md#options) as this rule also supports the same `commonjs` and `amd` flags. However, these flags only impact which import types are _linted_; the
import/export infrastructure only registers `import` statements in dependencies, so
cycles created by `require` within imported modules may not be detected.

#### `maxDepth`

There is a `maxDepth` option available to prevent full expansion of very deep dependency trees:

```js
/*eslint import/no-unresolved: [2, { maxDepth: 1 }]*/

// dep-c.js
import './dep-a.js'

// dep-b.js
import './dep-c.js'

export function b() { /* ... */ }

// dep-a.js
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.

## When Not To Use It

This rule is computationally expensive. If you are pressed for lint time, or don't
think you have an issue with dependency cycles, you may not want this rule enabled.
This rule is comparatively computationally expensive. If you are pressed for lint
time, or don't think you have an issue with dependency cycles, you may not want
this rule enabled.

## Further Reading

Expand Down
18 changes: 16 additions & 2 deletions tests/src/rules/no-cycle.js
Expand Up @@ -11,7 +11,7 @@ const test = def => _test(Object.assign(def, {
filename: testFilePath('./cycles/depth-zero.js'),
}))

// describe.only("no-cycle", () => {
describe.only("no-cycle", () => {
ruleTester.run('no-cycle', rule, {
valid: [
// this rule doesn't care if the cycle length is 0
Expand All @@ -32,12 +32,21 @@ ruleTester.run('no-cycle', rule, {
code: 'var bar = require("./bar")',
filename: '<text>',
}),
test({
code: 'import { foo } from "./depth-two"',
options: [{ maxDepth: 1 }],
}),
],
invalid: [
test({
code: 'import { foo } from "./depth-one"',
errors: [error(`Dependency cycle detected.`)],
}),
test({
code: 'import { foo } from "./depth-one"',
options: [{ maxDepth: 1 }],
errors: [error(`Dependency cycle detected.`)],
}),
test({
code: 'const { foo } = require("./depth-one")',
errors: [error(`Dependency cycle detected.`)],
Expand All @@ -57,6 +66,11 @@ ruleTester.run('no-cycle', rule, {
code: 'import { foo } from "./depth-two"',
errors: [error(`Dependency cycle via ./depth-one:1`)],
}),
test({
code: 'import { foo } from "./depth-two"',
options: [{ maxDepth: 2 }],
errors: [error(`Dependency cycle via ./depth-one:1`)],
}),
test({
code: 'const { foo } = require("./depth-two")',
errors: [error(`Dependency cycle via ./depth-one:1`)],
Expand All @@ -72,4 +86,4 @@ ruleTester.run('no-cycle', rule, {
}),
],
})
// })
})

0 comments on commit d81f48a

Please sign in to comment.