From 49218fa4385c43adc2fa5a4974af053fdc1767aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 27 Jul 2022 08:35:52 +0200 Subject: [PATCH] [Fix] `no-cycle`: add ExportNamedDeclaration statements to dependencies Fixes #2461 --- src/ExportMap.js | 44 +++++++++++--------- tests/files/cycles/es6/depth-one-reexport.js | 1 + tests/src/rules/no-cycle.js | 5 +++ 3 files changed, 31 insertions(+), 19 deletions(-) create mode 100644 tests/files/cycles/es6/depth-one-reexport.js diff --git a/src/ExportMap.js b/src/ExportMap.js index e18797a4d7..f26547ebe3 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -497,6 +497,28 @@ ExportMap.parse = function (path, content, context) { m.reexports.set(s.exported.name, { local, getImport: () => resolveImport(nsource) }); } + function captureDependencyWithSpecifiers(n) { + // import type { Foo } (TS and Flow) + const declarationIsType = n.importKind === 'type'; + // import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and + // shouldn't be considered to be just importing types + let specifiersOnlyImportingTypes = n.specifiers.length; + const importedSpecifiers = new Set(); + n.specifiers.forEach(specifier => { + if (supportedImportTypes.has(specifier.type)) { + importedSpecifiers.add(specifier.type); + } + if (specifier.type === 'ImportSpecifier') { + importedSpecifiers.add(specifier.imported.name || specifier.imported.value); + } + + // import { type Foo } (Flow) + specifiersOnlyImportingTypes = + specifiersOnlyImportingTypes && specifier.importKind === 'type'; + }); + captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers); + } + function captureDependency({ source }, isOnlyImportingTypes, importedSpecifiers = new Set()) { if (source == null) return null; @@ -583,25 +605,7 @@ ExportMap.parse = function (path, content, context) { // capture namespaces in case of later export if (n.type === 'ImportDeclaration') { - // import type { Foo } (TS and Flow) - const declarationIsType = n.importKind === 'type'; - // import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and - // shouldn't be considered to be just importing types - let specifiersOnlyImportingTypes = n.specifiers.length; - const importedSpecifiers = new Set(); - n.specifiers.forEach(specifier => { - if (supportedImportTypes.has(specifier.type)) { - importedSpecifiers.add(specifier.type); - } - if (specifier.type === 'ImportSpecifier') { - importedSpecifiers.add(specifier.imported.name || specifier.imported.value); - } - - // import { type Foo } (Flow) - specifiersOnlyImportingTypes = - specifiersOnlyImportingTypes && specifier.importKind === 'type'; - }); - captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers); + captureDependencyWithSpecifiers(n); const ns = n.specifiers.find(s => s.type === 'ImportNamespaceSpecifier'); if (ns) { @@ -611,6 +615,8 @@ ExportMap.parse = function (path, content, context) { } if (n.type === 'ExportNamedDeclaration') { + captureDependencyWithSpecifiers(n); + // capture declaration if (n.declaration != null) { switch (n.declaration.type) { diff --git a/tests/files/cycles/es6/depth-one-reexport.js b/tests/files/cycles/es6/depth-one-reexport.js new file mode 100644 index 0000000000..df509fa51c --- /dev/null +++ b/tests/files/cycles/es6/depth-one-reexport.js @@ -0,0 +1 @@ +export { foo } from "../depth-zero"; diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index ad29292c23..233cae613b 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -166,6 +166,11 @@ ruleTester.run('no-cycle', rule, { errors: [error(`Dependency cycle detected.`)], options: [{ ...opts, amd: true }], }), + test({ + code: `import { foo } from "./${testDialect}/depth-one-reexport"`, + options: [{ ...opts }], + errors: [error(`Dependency cycle detected.`)], + }), test({ code: `import { foo } from "./${testDialect}/depth-two"`, options: [{ ...opts }],