Skip to content

Commit

Permalink
[Fix] no-cycle: add ExportNamedDeclaration statements to dependencies
Browse files Browse the repository at this point in the history
Fixes #2461
  • Loading branch information
BenoitZugmeyer authored and ljharb committed Jul 27, 2022
1 parent 72824c7 commit 116af31
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 19 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [`no-restricted-paths`]: use `Minimatch.match` instead of `minimatch` to comply with Windows Native paths ([#2466], thanks [@AdriAt360])
- [`order`]: require with member expression could not be fixed if alphabetize.order was used ([#2490], thanks [@msvab])
- [`order`]: leave more space in rankings for consecutive path groups ([#2506], thanks [@Pearce-Ropion])
- [`no-cycle`]: add ExportNamedDeclaration statements to dependencies ([#2511], thanks [@BenoitZugmeyer])

### Changed
- [Tests] `named`: Run all TypeScript test ([#2427], thanks [@ProdigySim])
Expand Down Expand Up @@ -1001,6 +1002,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#2531]: https://github.com/import-js/eslint-plugin-import/pull/2531
[#2511]: https://github.com/import-js/eslint-plugin-import/pull/2511
[#2506]: https://github.com/import-js/eslint-plugin-import/pull/2506
[#2503]: https://github.com/import-js/eslint-plugin-import/pull/2503
[#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490
Expand Down Expand Up @@ -1529,6 +1531,7 @@ for info on changes for earlier releases.
[@beatrizrezener]: https://github.com/beatrizrezener
[@benmosher]: https://github.com/benmosher
[@benmunro]: https://github.com/benmunro
[@BenoitZugmeyer]: https://github.com/BenoitZugmeyer
[@bicstone]: https://github.com/bicstone
[@Blasz]: https://github.com/Blasz
[@bmish]: https://github.com/bmish
Expand Down
42 changes: 23 additions & 19 deletions src/ExportMap.js
Expand Up @@ -501,6 +501,26 @@ 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 > 0;
const importedSpecifiers = new Set();
n.specifiers.forEach(specifier => {
if (specifier.type === 'ImportSpecifier') {
importedSpecifiers.add(specifier.imported.name || specifier.imported.value);
} else if (supportedImportTypes.has(specifier.type)) {
importedSpecifiers.add(specifier.type);
}

// 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;

Expand Down Expand Up @@ -587,25 +607,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) {
Expand All @@ -615,6 +617,8 @@ ExportMap.parse = function (path, content, context) {
}

if (n.type === 'ExportNamedDeclaration') {
captureDependencyWithSpecifiers(n);

// capture declaration
if (n.declaration != null) {
switch (n.declaration.type) {
Expand Down
1 change: 1 addition & 0 deletions tests/files/cycles/es6/depth-one-reexport.js
@@ -0,0 +1 @@
export { foo } from "../depth-zero";
5 changes: 5 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -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 }],
Expand Down

0 comments on commit 116af31

Please sign in to comment.