Skip to content

Commit

Permalink
[Fix] no-cycle/extensions: fix isExternalModule usage
Browse files Browse the repository at this point in the history
  • Loading branch information
JEROMEH authored and ljharb committed Mar 24, 2020
1 parent 196d655 commit e22fc53
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 8 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-unused-modules`]: make type imports mark a module as used (fixes #1924) ([#1974], thanks [@cherryblossom000])
- [`no-cycle`]: fix perf regression ([#1944], thanks [@Blasz])
- [`first`]: fix handling of `import = require` ([#1963], thanks [@MatthiasKunnen])
- [`no-cycle`]/[`extensions`]: fix isExternalModule usage ([#1696], thanks [@paztis])

### Changed
- [Generic Import Callback] Make callback for all imports once in rules ([#1237], thanks [@ljqx])
Expand Down Expand Up @@ -788,6 +789,7 @@ for info on changes for earlier releases.
[#1719]: https://github.com/benmosher/eslint-plugin-import/pull/1719
[#1704]: https://github.com/benmosher/eslint-plugin-import/issues/1704
[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702
[#1696]: https://github.com/benmosher/eslint-plugin-import/pull/1696
[#1691]: https://github.com/benmosher/eslint-plugin-import/pull/1691
[#1690]: https://github.com/benmosher/eslint-plugin-import/pull/1690
[#1689]: https://github.com/benmosher/eslint-plugin-import/pull/1689
Expand Down Expand Up @@ -1310,3 +1312,4 @@ for info on changes for earlier releases.
[@swernerx]: https://github.com/swernerx
[@fsmaia]: https://github.com/fsmaia
[@MatthiasKunnen]: https://github.com/MatthiasKunnen
[@paztis]: https://github.com/paztis
5 changes: 4 additions & 1 deletion src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ function isSubpath(subpath, path) {
(right >= normPath.length || normPath[right] === '/');
}

const externalModuleRegExp = /^\w/;
const externalModuleRegExp = /^(?:\w|@)/;
export function isExternalModule(name, settings, path) {
if (arguments.length < 3) {
throw new TypeError('isExternalModule: name, settings, and path are all required');
}
return externalModuleRegExp.test(name) && isExternalPath(path, name, settings);
}

Expand Down
7 changes: 5 additions & 2 deletions src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,11 @@ module.exports = {
const extension = path.extname(resolvedPath || importPath).substring(1);

// determine if this is a module
const isPackage = isExternalModule(importPath, context.settings)
|| isScoped(importPath);
const isPackage = isExternalModule(
importPath,
context.settings,
resolve(importPath, context)
) || isScoped(importPath);

if (!extension || !importPath.endsWith(`.${extension}`)) {
const extensionRequired = isUseOfExtensionRequired(extension, isPackage);
Expand Down
7 changes: 6 additions & 1 deletion src/rules/no-cycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* @author Ben Mosher
*/

import resolve from 'eslint-module-utils/resolve';
import Exports from '../ExportMap';
import { isExternalModule } from '../core/importType';
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor';
Expand Down Expand Up @@ -41,7 +42,11 @@ module.exports = {

const options = context.options[0] || {};
const maxDepth = typeof options.maxDepth === 'number' ? options.maxDepth : Infinity;
const ignoreModule = (name) => options.ignoreExternal ? isExternalModule(name) : false;
const ignoreModule = (name) => options.ignoreExternal && isExternalModule(
name,
context.settings,
resolve(name, context)
);

function checkSourceValue(sourceNode, importer) {
if (ignoreModule(sourceNode.value)) {
Expand Down
29 changes: 29 additions & 0 deletions tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,5 +547,34 @@ ruleTester.run('extensions', rule, {
},
],
}),
test({
code: 'import foo from "@/ImNotAScopedModule.js"',
options: ['never'],
errors: [
{
message: 'Unexpected use of file extension "js" for "@/ImNotAScopedModule.js"',
line: 1,
},
],
}),
test({
code: `
import _ from 'lodash';
import m from '@test-scope/some-module/index.js';
import bar from './bar';
`,
options: ['never'],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['node_modules', 'symlinked-module'],
},
errors: [
{
message: 'Unexpected use of file extension "js" for "@test-scope/some-module/index.js"',
line: 3,
},
],
}),
],
});
8 changes: 4 additions & 4 deletions tests/src/rules/no-cycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ ruleTester.run('no-cycle', rule, {
options: [{ ignoreExternal: true }],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
'import/external-module-folders': ['cycles/external'],
},
}),
test({
code: 'import { foo } from "./external-depth-two"',
options: [{ ignoreExternal: true }],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
'import/external-module-folders': ['cycles/external'],
},
}),
test({
Expand Down Expand Up @@ -84,15 +84,15 @@ ruleTester.run('no-cycle', rule, {
errors: [error(`Dependency cycle detected.`)],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
'import/external-module-folders': ['cycles/external'],
},
}),
test({
code: 'import { foo } from "./external-depth-two"',
errors: [error(`Dependency cycle via cycles/external/depth-one:1`)],
settings: {
'import/resolver': 'webpack',
'import/external-module-folders': ['external'],
'import/external-module-folders': ['cycles/external'],
},
}),
test({
Expand Down

0 comments on commit e22fc53

Please sign in to comment.