Skip to content

Commit

Permalink
[Fix] Fix interpreting some external modules being interpreted as int…
Browse files Browse the repository at this point in the history
…ernal modules

Fixes import-js#793.

 - Add skipped test to expect scoped internal packages to be "internal"
  • Loading branch information
ephys authored and ljharb committed Apr 6, 2017
1 parent 083bb47 commit e9544f8
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
### Fixed
- [`order`]: Fix interpreting some external modules being interpreted as internal modules ([#793], [#794] thanks [@ephys])


## [2.16.0] - 2019-01-29
Expand Down Expand Up @@ -538,6 +540,7 @@ for info on changes for earlier releases.
[#843]: https://github.com/benmosher/eslint-plugin-import/pull/843
[#871]: https://github.com/benmosher/eslint-plugin-import/pull/871
[#797]: https://github.com/benmosher/eslint-plugin-import/pull/797
[#794]: https://github.com/benmosher/eslint-plugin-import/pull/794
[#744]: https://github.com/benmosher/eslint-plugin-import/pull/744
[#742]: https://github.com/benmosher/eslint-plugin-import/pull/742
[#737]: https://github.com/benmosher/eslint-plugin-import/pull/737
Expand Down Expand Up @@ -615,6 +618,7 @@ for info on changes for earlier releases.
[#717]: https://github.com/benmosher/eslint-plugin-import/issues/717
[#686]: https://github.com/benmosher/eslint-plugin-import/issues/686
[#671]: https://github.com/benmosher/eslint-plugin-import/issues/671
[#793]: https://github.com/benmosher/eslint-plugin-import/issues/793
[#660]: https://github.com/benmosher/eslint-plugin-import/issues/660
[#653]: https://github.com/benmosher/eslint-plugin-import/issues/653
[#627]: https://github.com/benmosher/eslint-plugin-import/issues/627
Expand Down Expand Up @@ -804,3 +808,4 @@ for info on changes for earlier releases.
[@kirill-konshin]: https://github.com/kirill-konshin
[@asapach]: https://github.com/asapach
[@sergei-startsev]: https://github.com/sergei-startsev
[@ephys]: https://github.com/ephys
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
"eslint-import-resolver-typescript": "^1.0.2",
"eslint-import-resolver-webpack": "file:./resolvers/webpack",
"eslint-module-utils": "file:./utils",
"eslint-import-test-order-redirect": "file:./tests/files/order-redirect",
"@eslint/import-test-order-redirect-scoped": "file:./tests/files/order-redirect-scoped",
"eslint-plugin-import": "2.x",
"gulp": "^3.9.1",
"gulp-babel": "6.1.2",
Expand Down
6 changes: 5 additions & 1 deletion src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ export function isBuiltIn(name, settings) {

function isExternalPath(path, name, settings) {
const folders = (settings && settings['import/external-module-folders']) || ['node_modules']
return !path || folders.some(folder => -1 < path.indexOf(join(folder, name)))

// 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)))
}

const externalModuleRegExp = /^\w/
Expand Down
1 change: 1 addition & 0 deletions tests/files/@importType/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/* for importType test, just needs to exist */
5 changes: 5 additions & 0 deletions tests/files/order-redirect-scoped/module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "order-redirect-module",
"private": true,
"main": "../other-module/file.js"
}
Empty file.
5 changes: 5 additions & 0 deletions tests/files/order-redirect-scoped/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "@eslint/import-test-order-redirect-scoped",
"version": "1.0.0",
"private": true
}
5 changes: 5 additions & 0 deletions tests/files/order-redirect/module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "order-redirect-module",
"private": true,
"main": "../other-module/file.js"
}
Empty file.
5 changes: 5 additions & 0 deletions tests/files/order-redirect/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "eslint-import-test-order-redirect",
"version": "1.0.0",
"private": true
}
10 changes: 10 additions & 0 deletions tests/src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,21 @@ describe('importType(name)', function () {
expect(importType('@some-thing/something/some-directory/someModule.js', context)).to.equal('external')
})

it("should return 'external' for external modules that redirect to its parent module using package.json", function() {
expect(importType('eslint-import-test-order-redirect/module', context)).to.equal('external')
expect(importType('@eslint/import-test-order-redirect-scoped/module', context)).to.equal('external')
})

it("should return 'internal' for non-builtins resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
expect(importType('importType', pathContext)).to.equal('internal')
})

it.skip("should return 'internal' for scoped packages resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
expect(importType('@importType/index', pathContext)).to.equal('internal')
})

it("should return 'parent' for internal modules that go through the parent", function() {
expect(importType('../foo', context)).to.equal('parent')
expect(importType('../../foo', context)).to.equal('parent')
Expand Down

0 comments on commit e9544f8

Please sign in to comment.