From 498b1024e616636385a62a44202b3eb1d0da7bac Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Fri, 29 Oct 2021 09:56:08 -0700 Subject: [PATCH 1/2] [Refactor] `importType`: combine redundant `isScoped` and `isScopedModule` --- CHANGELOG.md | 1 + src/core/importType.js | 4 ---- src/rules/extensions.js | 6 +++--- tests/src/core/importType.js | 9 +-------- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ca12a1b..35046f0f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Changed - [Docs] [`order`]: add type to the default groups ([#2272], [@charpeni]) - [readme] Add note to TypeScript docs to install appropriate resolver ([#2279], [@johnthagen]) +- [Refactor] `importType`: combine redundant `isScoped` and `isScopedModule` ([@ljharb]) ## [2.25.2] - 2021-10-12 diff --git a/src/core/importType.js b/src/core/importType.js index 6e4ac9a4d..e16a439e9 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -101,10 +101,6 @@ function typeTest(name, context, path) { return 'unknown'; } -export function isScopedModule(name) { - return name.indexOf('@') === 0 && !name.startsWith('@/'); -} - export default function resolveImportType(name, context) { return typeTest(name, context, resolve(name, context)); } diff --git a/src/rules/extensions.js b/src/rules/extensions.js index c22d1f87b..677a0a095 100644 --- a/src/rules/extensions.js +++ b/src/rules/extensions.js @@ -1,7 +1,7 @@ import path from 'path'; import resolve from 'eslint-module-utils/resolve'; -import { isBuiltIn, isExternalModule, isScoped, isScopedModule } from '../core/importType'; +import { isBuiltIn, isExternalModule, isScoped } from '../core/importType'; import moduleVisitor from 'eslint-module-utils/moduleVisitor'; import docsUrl from '../docsUrl'; @@ -131,7 +131,7 @@ module.exports = { const slashCount = file.split('/').length - 1; if (slashCount === 0) return true; - if (isScopedModule(file) && slashCount <= 1) return true; + if (isScoped(file) && slashCount <= 1) return true; return false; } @@ -161,7 +161,7 @@ module.exports = { importPath, context.settings, resolve(importPath, context), - context + context, ) || isScoped(importPath); if (!extension || !importPath.endsWith(`.${extension}`)) { diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index 0dcd5266b..5f15f230a 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -1,7 +1,7 @@ import { expect } from 'chai'; import * as path from 'path'; -import importType, { isExternalModule, isScopedModule, isScoped } from 'core/importType'; +import importType, { isExternalModule, isScoped } from 'core/importType'; import { testContext, testFilePath } from '../utils'; @@ -239,13 +239,6 @@ describe('importType(name)', function () { }, 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true); }); - it('correctly identifies scoped modules with `isScopedModule`', () => { - expect(isScopedModule('@/abc')).to.equal(false); - expect(isScopedModule('@/abc/def')).to.equal(false); - expect(isScopedModule('@a/abc')).to.equal(true); - expect(isScopedModule('@a/abc/def')).to.equal(true); - }); - it('correctly identifies scoped modules with `isScoped`', () => { expect(isScoped('@/abc')).to.equal(false); expect(isScoped('@/abc/def')).to.equal(false); From 6682e9a492f1a138e0a32d11d3a65feecfec3aee Mon Sep 17 00:00:00 2001 From: Bernhard Jahn Date: Fri, 29 Oct 2021 17:40:35 +0200 Subject: [PATCH 2/2] [Fix] `importType`: fix `isExternalModule` calculation Fixes #2258 --- CHANGELOG.md | 4 ++++ src/core/importType.js | 2 +- tests/src/core/importType.js | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35046f0f8..3821c7e6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Fixed - [`extensions`]: ignore unresolveable type-only imports ([#2270], [#2271], [@jablko]) +- `importType`: fix `isExternalModule` calculation ([#2282], [@mx-bernhard]) ### Changed - [Docs] [`order`]: add type to the default groups ([#2272], [@charpeni]) @@ -937,6 +938,8 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#2282]: https://github.com/import-js/eslint-plugin-import/pull/2282 +[#2279]: https://github.com/import-js/eslint-plugin-import/pull/2279 [#2272]: https://github.com/import-js/eslint-plugin-import/pull/2272 [#2271]: https://github.com/import-js/eslint-plugin-import/pull/2271 [#2270]: https://github.com/import-js/eslint-plugin-import/pull/2270 @@ -1543,6 +1546,7 @@ for info on changes for earlier releases. [@MikeyBeLike]: https://github.com/MikeyBeLike [@mplewis]: https://github.com/mplewis [@mrmckeb]: https://github.com/mrmckeb +[@mx-bernhard]: https://github.com/mx-bernhard [@nickofthyme]: https://github.com/nickofthyme [@nicolashenry]: https://github.com/nicolashenry [@noelebrun]: https://github.com/noelebrun diff --git a/src/core/importType.js b/src/core/importType.js index e16a439e9..085ce6582 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -29,7 +29,7 @@ export function isExternalModule(name, settings, path, context) { if (arguments.length < 4) { throw new TypeError('isExternalModule: name, settings, path, and context are all required'); } - return isModule(name) && isExternalPath(name, settings, path, getContextPackagePath(context)); + return (isModule(name) || isScoped(name)) && isExternalPath(name, settings, path, getContextPackagePath(context)); } export function isExternalModuleMain(name, settings, path, context) { diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index 5f15f230a..528377e69 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -234,11 +234,21 @@ describe('importType(name)', function () { it('`isExternalModule` works with windows directory separator', function () { const context = testContext(); expect(isExternalModule('foo', {}, 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true); + expect(isExternalModule('@foo/bar', {}, 'E:\\path\\to\\node_modules\\@foo\\bar', context)).to.equal(true); expect(isExternalModule('foo', { 'import/external-module-folders': ['E:\\path\\to\\node_modules'], }, 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true); }); + it('`isExternalModule` works with unix directory separator', function () { + const context = testContext(); + expect(isExternalModule('foo', {}, '/path/to/node_modules/foo', context)).to.equal(true); + expect(isExternalModule('@foo/bar', {}, '/path/to/node_modules/@foo/bar', context)).to.equal(true); + expect(isExternalModule('foo', { + 'import/external-module-folders': ['/path/to/node_modules'], + }, '/path/to/node_modules/foo', context)).to.equal(true); + }); + it('correctly identifies scoped modules with `isScoped`', () => { expect(isScoped('@/abc')).to.equal(false); expect(isScoped('@/abc/def')).to.equal(false);