Repository: import-js/eslint-plugin-import (js)

Issue:
Allow cyclic dependencies for `import/no-cycle` iff mixing static/dynamic imports
## Request

Add a new option for `import/no-cycle` to allow/warn/error about cyclic dependencies if, and only if, the cycle involves dynamic imports.

## Problem Statement

There exists a use case for web apps where some modules are stored in a form of global state and then other modules import that state to update it. For example, an (overly simplistic/half-pseudo code) purpose for this might look something like:

```javascript
// state.js
import Comp1 from './Comp1';
import Comp2 from './Comp2';
import Comp3 from './Comp3';

const routeModuleMap = {
    '/path1': Comp1,
    '/path2': Comp2,
    '/path3': Comp3,
};

const routeOrder = [
    '/path1',
    '/path2',
    '/path3',
];

let state = 0;

export function getActiveModule() {
    return routeModuleMap[location.pathname];
}

export function setActiveModule(dx = 1) {
    if (dx === 0) {
        return;
    }

    if (dx < 0) {
        state = Math.max(0, state + dx);
    }

    if (dx > 0) {
        state = Math.min(routeOrder.length - 1, state + dx);
    }

    history.pushState(null, null, routeOrder[state]);
}

export default const Context = React.createContext({ getActiveModule, setActiveModule });


// parent.jsx
import Context, { getActiveModule, setActiveModule } from './state';

export default function App() {
    const Component = getActiveModule();

    return (
        <Context.Provider value={{ getActiveModule, setActiveModule }}>
            <Component />
        </Context.Provider>
    );
}


// Comp1.jsx
import Context from './state';

export default Comp1() {
    const { setActiveModule } = useContext(Context);

    return (
        <button onClick={() => setActiveModule()}>
            Go to next page
        </button>
    );
}
```

Obviously this has a circular dependency, so when bundled, it might result in `CompN is not defined` errors.
However, that can easily be resolved by mixing static and dynamic imports, which return promises instead of the code itself. This means that all files are parsed/run _before_ any promises resolve, meaning that we never have `X is not defined` errors.

i.e.

```javascript
// old
import Comp1 from './Comp1';
import Comp2 from './Comp2';
import Comp3 from './Comp3';

const routeModuleMap = {
    '/path1': Comp1,
    '/path2': Comp2,
    '/path3': Comp3,
};


// new
const routeModuleMap = {
    '/path1': React.lazy(() => import('./Comp1')),
    '/path2': React.lazy(() => import('./Comp2')),
    '/path3': React.lazy(() => import('./Comp3')),
};
```

While this is a very simple and contrived example, there are cases where some sort of global state links to modules, and each module needs to modify that global state. In these cases, mixing static with dynamic imports solves the problem of code not being defined because your bundler of choice converts `A --> B --> A` to `A --> B --> Promise.resolve(A)`.

Of course, having circular dependencies in your code is an anti-pattern and reflects brittle code that should still be refactored. This is a great plugin/rule to do exactly that: fix old code and guide new code standards.
But for gigantic repositories, those can't all be fixed at once, especially when the code is in more complex global state. For those specific situations, it'd be really helpful to be able to toggle dynamic-import-cycles using dynamic imports from allow --> warn --> error gradually as they are addressed.
This would mean **errors are still shown** for static import cycles, protecting new code while old code is refactored.

Pull request:
[New] `no-cycle`: add option to allow cycle via dynamic import
Hi,

as discussed in #2265, I added a new option, `allowUnsafeDynamicCyclicDependency`. I've tested all cases I could.

Unfortunately, I did not managed to write tests using the `TS_OLD` parser. I don't know if this is important.

Feedbacks are welcome ! Have a nice day

Fixes #2265.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cb455b42..072911de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 - [`no-named-default`, `no-default-export`, `prefer-default-export`, `no-named-export`, `export`, `named`, `namespace`, `no-unused-modules`]: support arbitrary module namespace names ([#2358], thanks [@sosukesuzuki])
 - [`no-dynamic-require`]: support dynamic import with espree ([#2371], thanks [@sosukesuzuki])
 - [`no-relative-packages`]: add fixer ([#2381], thanks [@forivall])
+- [`no-cycle`]: add `allowUnsafeDynamicCyclicDependency` option ([#2387], thanks [@GerkinDev])
 
 ### Fixed
 - [`default`]: `typescript-eslint-parser`: avoid a crash on exporting as namespace (thanks [@ljharb])
diff --git a/docs/rules/no-cycle.md b/docs/rules/no-cycle.md
index 7f1b4255..70b2ceb9 100644
--- a/docs/rules/no-cycle.md
+++ b/docs/rules/no-cycle.md
@@ -75,6 +75,22 @@ 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.
 
+#### `allowUnsafeDynamicCyclicDependency`
+
+This option disable reporting of errors if a cycle is detected with at least one dynamic import.
+
+```js
+// bar.js
+import { foo } from './foo';
+export const bar = foo;
+
+// foo.js
+export const foo = 'Foo';
+export function getBar() { return import('./bar'); }
+```
+
+> Cyclic dependency are **always** a dangerous anti-pattern as discussed extensively in [#2265](https://github.com/import-js/eslint-plugin-import/issues/2265). Please be extra careful about using this option.
+
 ## When Not To Use It
 
 This rule is comparatively computationally expensive. If you are pressed for lint
diff --git a/src/ExportMap.js b/src/ExportMap.js
index d75c7ecd..e18797a4 100644
--- a/src/ExportMap.js
+++ b/src/ExportMap.js
@@ -395,6 +395,7 @@ ExportMap.parse = function (path, content, context) {
           loc: source.loc,
         },
         importedSpecifiers,
+        dynamic: true,
       }]),
     });
   }
diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js
index e61c3be2..0aa36268 100644
--- a/src/rules/no-cycle.js
+++ b/src/rules/no-cycle.js
@@ -33,6 +33,11 @@ module.exports = {
         type: 'boolean',
         default: false,
       },
+      allowUnsafeDynamicCyclicDependency: {
+        description: 'Allow cyclic dependency if there is at least one dynamic import in the chain',
+        type: 'boolean',
+        default: false,
+      },
     })],
   },
 
@@ -52,6 +57,13 @@ module.exports = {
       if (ignoreModule(sourceNode.value)) {
         return; // ignore external modules
       }
+      if (options.allowUnsafeDynamicCyclicDependency && (
+        // Ignore `import()`
+        importer.type === 'ImportExpression' ||
+        // `require()` calls are always checked (if possible)
+        (importer.type === 'CallExpression' && importer.callee.name !== 'require'))) {
+        return; // cycle via dynamic import allowed by config
+      }
 
       if (
         importer.type === 'ImportDeclaration' && (
@@ -89,6 +101,12 @@ module.exports = {
             // Ignore only type imports
             !isOnlyImportingTypes,
           );
+          
+          /*
+          If cyclic dependency is allowed via dynamic import, skip checking if any module is imported dynamically
+          */
+          if (options.allowUnsafeDynamicCyclicDependency && toTraverse.some(d => d.dynamic)) return;
+
           /*
           Only report as a cycle if there are any import declarations that are considered by
           the rule. For example:


Test patch:
diff --git a/tests/files/cycles/es6/depth-one-dynamic.js b/tests/files/cycles/es6/depth-one-dynamic.js
new file mode 100644
index 00000000..32dd3db4
--- /dev/null
+++ b/tests/files/cycles/es6/depth-one-dynamic.js
@@ -0,0 +1,1 @@
+export const bar = () => import("../depth-zero").then(({foo}) => foo);
diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js
index 22e097dd..ad29292c 100644
--- a/tests/src/rules/no-cycle.js
+++ b/tests/src/rules/no-cycle.js
@@ -1,4 +1,4 @@
-import { parsers, test as _test, testFilePath } from '../utils';
+import { parsers, test as _test, testFilePath, testVersion as _testVersion } from '../utils';
 
 import { RuleTester } from 'eslint';
 import flatMap from 'array.prototype.flatmap';
@@ -11,6 +11,9 @@ const error = message => ({ message });
 const test = def => _test(Object.assign(def, {
   filename: testFilePath('./cycles/depth-zero.js'),
 }));
+const testVersion = (specifier, t) => _testVersion(specifier, () => Object.assign(t(), {
+  filename: testFilePath('./cycles/depth-zero.js'),
+}));
 
 const testDialects = ['es6'];
 
@@ -73,7 +76,28 @@ ruleTester.run('no-cycle', rule, {
         code: `import type { FooType, BarType } from "./${testDialect}/depth-one"`,
         parser: parsers.BABEL_OLD,
       }),
-    ]),
+      test({
+        code: `function bar(){ return import("./${testDialect}/depth-one"); } // #2265 1`,
+        options: [{ allowUnsafeDynamicCyclicDependency: true }],
+        parser: parsers.BABEL_OLD,
+      }),
+      test({
+        code: `import { foo } from "./${testDialect}/depth-one-dynamic"; // #2265 2`,
+        options: [{ allowUnsafeDynamicCyclicDependency: true }],
+        parser: parsers.BABEL_OLD,
+      }),
+    ].concat(parsers.TS_NEW ? [
+      test({
+        code: `function bar(){ return import("./${testDialect}/depth-one"); } // #2265 3`,
+        options: [{ allowUnsafeDynamicCyclicDependency: true }],
+        parser: parsers.TS_NEW,
+      }),
+      test({
+        code: `import { foo } from "./${testDialect}/depth-one-dynamic"; // #2265 4`,
+        options: [{ allowUnsafeDynamicCyclicDependency: true }],
+        parser: parsers.TS_NEW,
+      }),
+    ] : [])),
 
     test({
       code: 'import { bar } from "./flow-types"',
@@ -112,62 +136,83 @@ ruleTester.run('no-cycle', rule, {
       },
     }),
 
-    flatMap(testDialects, (testDialect) => [
+    // Ensure behavior does not change for those tests, with or without `
+    flatMap(testDialects, (testDialect) => flatMap([
+      {},
+      { allowUnsafeDynamicCyclicDependency: true },
+    ], (opts) => [
       test({
         code: `import { foo } from "./${testDialect}/depth-one"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle detected.`)],
       }),
       test({
         code: `import { foo } from "./${testDialect}/depth-one"`,
-        options: [{ maxDepth: 1 }],
+        options: [{ ...opts, maxDepth: 1 }],
         errors: [error(`Dependency cycle detected.`)],
       }),
       test({
         code: `const { foo } = require("./${testDialect}/depth-one")`,
         errors: [error(`Dependency cycle detected.`)],
-        options: [{ commonjs: true }],
+        options: [{ ...opts, commonjs: true }],
       }),
       test({
         code: `require(["./${testDialect}/depth-one"], d1 => {})`,
         errors: [error(`Dependency cycle detected.`)],
-        options: [{ amd: true }],
+        options: [{ ...opts, amd: true }],
       }),
       test({
         code: `define(["./${testDialect}/depth-one"], d1 => {})`,
         errors: [error(`Dependency cycle detected.`)],
-        options: [{ amd: true }],
+        options: [{ ...opts, amd: true }],
       }),
       test({
         code: `import { foo } from "./${testDialect}/depth-two"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle via ./depth-one:1`)],
       }),
       test({
         code: `import { foo } from "./${testDialect}/depth-two"`,
-        options: [{ maxDepth: 2 }],
+        options: [{ ...opts, maxDepth: 2 }],
         errors: [error(`Dependency cycle via ./depth-one:1`)],
       }),
       test({
         code: `const { foo } = require("./${testDialect}/depth-two")`,
         errors: [error(`Dependency cycle via ./depth-one:1`)],
-        options: [{ commonjs: true }],
+        options: [{ ...opts, commonjs: true }],
       }),
       test({
         code: `import { two } from "./${testDialect}/depth-three-star"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
       }),
       test({
         code: `import one, { two, three } from "./${testDialect}/depth-three-star"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
       }),
       test({
         code: `import { bar } from "./${testDialect}/depth-three-indirect"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
       }),
       test({
         code: `import { bar } from "./${testDialect}/depth-three-indirect"`,
+        options: [{ ...opts }],
         errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
         parser: parsers.BABEL_OLD,
       }),
+      test({
+        code: `import { foo } from "./${testDialect}/depth-two"`,
+        options: [{ ...opts, maxDepth: Infinity }],
+        errors: [error(`Dependency cycle via ./depth-one:1`)],
+      }),
+      test({
+        code: `import { foo } from "./${testDialect}/depth-two"`,
+        options: [{ ...opts, maxDepth: '∞' }],
+        errors: [error(`Dependency cycle via ./depth-one:1`)],
+      }),
+    ]).concat([
       test({
         code: `import("./${testDialect}/depth-three-star")`,
         errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
@@ -188,7 +233,29 @@ ruleTester.run('no-cycle', rule, {
         options: [{ maxDepth: '∞' }],
         errors: [error(`Dependency cycle via ./depth-one:1`)],
       }),
-    ]),
+      test({
+        code: `function bar(){ return import("./${testDialect}/depth-one"); } // #2265 5`,
+        errors: [error(`Dependency cycle detected.`)],
+        parser: parsers.BABEL_OLD,
+      }),
+    ]).concat(
+      testVersion('> 3', () => ({ // Dynamic import is not properly caracterized with eslint < 4
+        code: `import { foo } from "./${testDialect}/depth-one-dynamic"; // #2265 6`,
+        errors: [error(`Dependency cycle detected.`)],
+        parser: parsers.BABEL_OLD,
+      })),
+    ).concat(parsers.TS_NEW ? [
+      test({
+        code: `function bar(){ return import("./${testDialect}/depth-one"); } // #2265 7`,
+        errors: [error(`Dependency cycle detected.`)],
+        parser: parsers.TS_NEW,
+      }),
+      test({
+        code: `import { foo } from "./${testDialect}/depth-one-dynamic"; // #2265 8`,
+        errors: [error(`Dependency cycle detected.`)],
+        parser: parsers.TS_NEW,
+      }),
+    ] : [])),
 
     test({
       code: 'import { bar } from "./flow-types-depth-one"',