Fix import type determination for monorepo setup w/ webpack resolver

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
@@ -636,6 +641,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
@@ -1069,3 +1075,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

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`
 

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",

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/

tests/files/symlinked-module/index.js

@@ -0,0 +1 @@
+export default {}

tests/files/symlinked-module/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "@test-scope/some-module",
+  "version": "1.0.0",
+  "private": true
+}

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()
@@ -48,7 +48,7 @@ describe('importType(name)', function () {
     expect(importType('importType', pathContext)).to.equal('internal')
   })
 
-  it.skip("should return 'internal' for scoped packages resolved outside of node_modules", function () {
+  it("should return 'internal' for scoped packages resolved outside of node_modules", function () {
     const pathContext = testContext({ 'import/resolver': { node: { paths: [pathToTestFiles] } } })
     expect(importType('@importType/index', pathContext)).to.equal('internal')
   })
@@ -145,4 +145,83 @@ describe('importType(name)', function () {
     const foldersContext = testContext({ 'import/external-module-folders': ['node_modules'] })
     expect(importType('resolve', foldersContext)).to.equal('external')
   })
+
+  it('returns "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/<package-name> but will point to the
+  // actual directory inside 'files' instead
+  it('returns "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('returns "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_2)).to.equal('internal')
+  })
+
+  it('returns "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('returns "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('returns "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('returns "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('returns "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')
+  })
 })

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: `