diff --git a/CHANGELOG.md b/CHANGELOG.md index e89efd1bee..3b913f7ee2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +### Fixed +- [`import/external-module-folders` setting] now correctly works with directories containing modules symlinked from `node_modules` ([#1605], thanks [@skozin]) + +### Changed +- [`import/external-module-folders` setting] behavior is more strict now: it will only match complete path segments ([#1605], thanks [@skozin]) ## [2.20.0] - 2020-01-10 ### Added @@ -638,6 +643,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#1605]: https://github.com/benmosher/eslint-plugin-import/pull/1605 [#1589]: https://github.com/benmosher/eslint-plugin-import/issues/1589 [#1586]: https://github.com/benmosher/eslint-plugin-import/pull/1586 [#1572]: https://github.com/benmosher/eslint-plugin-import/pull/1572 @@ -1071,3 +1077,4 @@ for info on changes for earlier releases. [@rsolomon]: https://github.com/rsolomon [@joaovieira]: https://github.com/joaovieira [@ivo-stefchev]: https://github.com/ivo-stefchev +[@skozin]: https://github.com/skozin diff --git a/README.md b/README.md index 814d5fc28d..2bd73a3ba3 100644 --- a/README.md +++ b/README.md @@ -339,7 +339,19 @@ Contribution of more such shared configs for other platforms are welcome! #### `import/external-module-folders` -An array of folders. Resolved modules only from those folders will be considered as "external". By default - `["node_modules"]`. Makes sense if you have configured your path or webpack to handle your internal paths differently and want to considered modules from some folders, for example `bower_components` or `jspm_modules`, as "external". +An array of folders. Resolved modules only from those folders will be considered as "external". By default - `["node_modules"]`. Makes sense if you have configured your path or webpack to handle your internal paths differently and want to consider modules from some folders, for example `bower_components` or `jspm_modules`, as "external". + +This option is also useful in a monorepo setup: list here all directories that contain monorepo's packages and they will be treated as external ones no matter which resolver is used. + +Each item in this array is either a folder's name, its subpath, or its absolute prefix path: + +- `jspm_modules` will match any file or folder named `jspm_modules` or which has a direct or non-direct parent named `jspm_modules`, e.g. `/home/me/project/jspm_modules` or `/home/me/project/jspm_modules/some-pkg/index.js`. + +- `packages/core` will match any path that contains these two segments, for example `/home/me/project/packages/core/src/utils.js`. + +- `/home/me/project/packages` will only match files and directories inside this directory, and the directory itself. + +Please note that incomplete names are not allowed here so `components` won't match `bower_components` and `packages/ui` won't match `packages/ui-utils` (but will match `packages/ui/utils`). #### `import/parsers` diff --git a/package.json b/package.json index 906cf8fa46..bfb70de07d 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "homepage": "https://github.com/benmosher/eslint-plugin-import", "devDependencies": { "@eslint/import-test-order-redirect-scoped": "file:./tests/files/order-redirect-scoped", + "@test-scope/some-module": "file:./tests/files/symlinked-module", "@typescript-eslint/parser": "1.10.3-alpha.13", "babel-cli": "^6.26.0", "babel-core": "^6.26.3", diff --git a/src/core/importType.js b/src/core/importType.js index 8ec01c29e8..d99f9a3481 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -1,5 +1,4 @@ import coreModules from 'resolve/lib/core' -import { join } from 'path' import resolve from 'eslint-module-utils/resolve' @@ -26,11 +25,19 @@ export function isBuiltIn(name, settings, path) { function isExternalPath(path, name, settings) { const folders = (settings && settings['import/external-module-folders']) || ['node_modules'] + return !path || folders.some(folder => isSubpath(folder, path)) +} - // extract the part before the first / (redux-saga/effects => redux-saga) - const packageName = name.match(/([^/]+)/)[0] - - return !path || folders.some(folder => -1 < path.indexOf(join(folder, packageName))) +function isSubpath(subpath, path) { + const normSubpath = subpath.replace(/[/]$/, '') + if (normSubpath.length === 0) { + return false + } + const left = path.indexOf(normSubpath) + const right = left + normSubpath.length + return left !== -1 && + (left === 0 || normSubpath[0] !== '/' && path[left - 1] === '/') && + (right >= path.length || path[right] === '/') } const externalModuleRegExp = /^\w/ diff --git a/tests/files/symlinked-module/index.js b/tests/files/symlinked-module/index.js new file mode 100644 index 0000000000..b1c6ea436a --- /dev/null +++ b/tests/files/symlinked-module/index.js @@ -0,0 +1 @@ +export default {} diff --git a/tests/files/symlinked-module/package.json b/tests/files/symlinked-module/package.json new file mode 100644 index 0000000000..722be5c3c4 --- /dev/null +++ b/tests/files/symlinked-module/package.json @@ -0,0 +1,5 @@ +{ + "name": "@test-scope/some-module", + "version": "1.0.0", + "private": true +} diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index f1e951cc47..b3343083c5 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -3,7 +3,7 @@ import * as path from 'path' import importType from 'core/importType' -import { testContext } from '../utils' +import { testContext, testFilePath } from '../utils' describe('importType(name)', function () { const context = testContext() @@ -145,4 +145,92 @@ describe('importType(name)', function () { const foldersContext = testContext({ 'import/external-module-folders': ['node_modules'] }) expect(importType('resolve', foldersContext)).to.equal('external') }) + + it("should return 'external' for a scoped symlinked module", function() { + const foldersContext = testContext({ + 'import/resolver': 'node', + 'import/external-module-folders': ['node_modules'], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('external') + }) + + // We're using Webpack resolver here since it resolves all symlinks, which means that + // directory path will not contain node_modules/ but will point to the + // actual directory inside 'files' instead + it("should return 'external' for a scoped module from a symlinked directory which name " + + "is contained in 'external-module-folders' (webpack resolver)", function() { + const foldersContext = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['symlinked-module'], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('external') + }) + + it("should return 'internal' for a scoped module from a symlinked directory which incomplete " + + "name is contained in 'external-module-folders' (webpack resolver)", function() { + const foldersContext_1 = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['symlinked-mod'], + }) + expect(importType('@test-scope/some-module', foldersContext_1)).to.equal('internal') + + const foldersContext_2 = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['linked-module'], + }) + expect(importType('@test-scope/some-module', foldersContext_1)).to.equal('internal') + }) + + it("should return 'external' for a scoped module from a symlinked directory which partial path " + + "is contained in 'external-module-folders' (webpack resolver)", function() { + const foldersContext = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['files/symlinked-module'], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('external') + }) + + it("should return 'internal' for a scoped module from a symlinked directory which partial path " + + "w/ incomplete segment is contained in 'external-module-folders' " + + "(webpack resolver)", function() { + const foldersContext_1 = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['files/symlinked-mod'], + }) + expect(importType('@test-scope/some-module', foldersContext_1)).to.equal('internal') + + const foldersContext_2 = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['les/symlinked-module'], + }) + expect(importType('@test-scope/some-module', foldersContext_2)).to.equal('internal') + }) + + it("should return 'external' for a scoped module from a symlinked directory which partial path " + + "ending w/ slash is contained in 'external-module-folders' (webpack resolver)", function() { + const foldersContext = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['files/symlinked-module/'], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('external') + }) + + it("should return 'internal' for a scoped module from a symlinked directory when " + + "'external-module-folders' contains an absolute path resembling directory's relative " + + "path (webpack resolver)", function() { + const foldersContext = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': ['/files/symlinked-module'], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('internal') + }) + + it("should return 'external' for a scoped module from a symlinked directory which absolute " + + "path is contained in 'external-module-folders' (webpack resolver)", function() { + const foldersContext = testContext({ + 'import/resolver': 'webpack', + 'import/external-module-folders': [testFilePath('symlinked-module')], + }) + expect(importType('@test-scope/some-module', foldersContext)).to.equal('external') + }) }) diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 8ba8b9f1eb..29ecc28e2d 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -298,7 +298,55 @@ ruleTester.run('order', rule, { ], }], }), + // Monorepo setup, using Webpack resolver, workspace folder name in external-module-folders + test({ + code: ` + import _ from 'lodash'; + import m from '@test-scope/some-module'; + + import bar from './bar'; + `, + options: [{ + 'newlines-between': 'always', + }], + settings: { + 'import/resolver': 'webpack', + 'import/external-module-folders': ['node_modules', 'symlinked-module'], + }, + }), + // Monorepo setup, using Webpack resolver, partial workspace folder path + // in external-module-folders + test({ + code: ` + import _ from 'lodash'; + import m from '@test-scope/some-module'; + + import bar from './bar'; + `, + options: [{ + 'newlines-between': 'always', + }], + settings: { + 'import/resolver': 'webpack', + 'import/external-module-folders': ['node_modules', 'files/symlinked-module'], + }, + }), + // Monorepo setup, using Node resolver (doesn't resolve symlinks) + test({ + code: ` + import _ from 'lodash'; + import m from '@test-scope/some-module'; + import bar from './bar'; + `, + options: [{ + 'newlines-between': 'always', + }], + settings: { + 'import/resolver': 'node', + 'import/external-module-folders': ['node_modules', 'files/symlinked-module'], + }, + }), // Option: newlines-between: 'always' test({ code: `