Skip to content

Commit

Permalink
[Fix] make 'import/order' work in a monorepo setup with scoped modules
Browse files Browse the repository at this point in the history
Fixes #1597
  • Loading branch information
skozin authored and ljharb committed Jan 11, 2020
1 parent 6af5772 commit 9fac546
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 7 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
14 changes: 13 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
17 changes: 12 additions & 5 deletions src/core/importType.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import coreModules from 'resolve/lib/core'
import { join } from 'path'

import resolve from 'eslint-module-utils/resolve'

Expand All @@ -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/
Expand Down
1 change: 1 addition & 0 deletions tests/files/symlinked-module/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default {}
5 changes: 5 additions & 0 deletions tests/files/symlinked-module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "@test-scope/some-module",
"version": "1.0.0",
"private": true
}
90 changes: 89 additions & 1 deletion tests/src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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/<package-name> 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')
})
})
48 changes: 48 additions & 0 deletions tests/src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down

0 comments on commit 9fac546

Please sign in to comment.