Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

import/no-cycle does not detect cycles when using named exports with a source #2461

Closed
BenoitZugmeyer opened this issue May 30, 2022 · 2 comments

Comments

@BenoitZugmeyer
Copy link
Contributor

Overview

I am facing an issue where the no-cycle rule fails to detect cycles when using named exports with a source (ex: export { bar } from "./bar.js"). The issue does not occur when using "export all" (ex: export * from "./bar.js").

How to reproduce

$ cat .eslintrc.js
module.exports = {
  plugins: ["import"],
  parserOptions: {
    ecmaVersion: "latest",
    sourceType: "module",
  },
  rules: {
    "import/no-cycle": "error",
  },
};

$ cat index.js
export { bar } from "./reexport.js";
export const foo = 1;

$ cat reexport.js
export { bar } from "./bar.js";

$ cat bar.js
import { foo } from "./index.js";
export const bar = foo + 1;

$ npx eslint .
(no output)

Actual result

The dependency cycle is not detected

Expected result

The dependency cycle is detected. For example, when changing reexport.js to an "export all", the dependency loop is correctly detected:

$ cat reexport.js
export * from "./bar.js"

$ npx eslint .
/.../index.js
  1:1  error  Dependency cycle via ./bar.js:1  import/no-cycle

✖ 1 problem (1 error, 0 warnings)

Fix suggestion

It seems we don't capture dependencies of ExportNamedDeclaration nodes. This patch seems to fix the issue:

diff --git src/ExportMap.js src/ExportMap.js
index e18797a..fdaecec 100644
--- src/ExportMap.js
+++ src/ExportMap.js
@@ -611,6 +611,9 @@ ExportMap.parse = function (path, content, context) {
     }
 
     if (n.type === 'ExportNamedDeclaration') {
+      const getter = captureDependency(n, n.exportKind === 'type');
+      if (getter) m.dependencies.add(getter);
+
       // capture declaration
       if (n.declaration != null) {
         switch (n.declaration.type) {
@JounQin
Copy link
Collaborator

JounQin commented Jul 20, 2022

Can you raise a PR and add the test case?

@BenoitZugmeyer
Copy link
Contributor Author

See #2511

@ljharb ljharb closed this as completed in 116af31 Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants