From b4bad0ed3c6b040093b9f365a3d1c9a4cdcdf4da Mon Sep 17 00:00:00 2001 From: jeffshaver Date: Sun, 24 Feb 2019 18:05:06 -0500 Subject: [PATCH 1/3] [Tests] fix a bunch of tests that were failing --- tests/src/rules/export.js | 6 +++--- tests/src/rules/extensions.js | 8 ++++---- tests/src/rules/named.js | 2 +- tests/src/rules/no-default-export.js | 5 +++-- tests/src/rules/no-named-export.js | 5 +++-- tests/src/rules/no-unresolved.js | 2 +- tests/src/rules/prefer-default-export.js | 3 +++ tests/src/utils.js | 4 ++-- 8 files changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/src/rules/export.js b/tests/src/rules/export.js index 84598677da..33f0121233 100644 --- a/tests/src/rules/export.js +++ b/tests/src/rules/export.js @@ -17,8 +17,8 @@ ruleTester.run('export', rule, { test({ code: 'export var [ foo, bar ] = array;' }), test({ code: 'export var { foo, bar } = object;' }), test({ code: 'export var [ foo, bar ] = array;' }), - test({ code: 'export { foo, foo as bar }' }), - test({ code: 'export { bar }; export * from "./export-all"' }), + test({ code: 'let foo; export { foo, foo as bar }' }), + test({ code: 'let bar; export { bar }; export * from "./export-all"' }), test({ code: 'export * from "./export-all"' }), test({ code: 'export * from "./does-not-exist"' }), @@ -62,7 +62,7 @@ ruleTester.run('export', rule, { // errors: ['Parsing error: Duplicate export \'foo\''], // }), test({ - code: 'export { foo }; export * from "./export-all"', + code: 'let foo; export { foo }; export * from "./export-all"', errors: ['Multiple exports of name \'foo\'.', 'Multiple exports of name \'foo\'.'], }), diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 8b816daa9e..d7b97bea0b 100644 --- a/tests/src/rules/extensions.js +++ b/tests/src/rules/extensions.js @@ -105,14 +105,14 @@ ruleTester.run('extensions', rule, { test({ code: [ 'export { foo } from "./foo.js"', - 'export { bar }', + 'let bar; export { bar }', ].join('\n'), options: [ 'always' ], }), test({ code: [ 'export { foo } from "./foo"', - 'export { bar }', + 'let bar; export { bar }', ].join('\n'), options: [ 'never' ], }), @@ -334,7 +334,7 @@ ruleTester.run('extensions', rule, { test({ code: [ 'export { foo } from "./foo"', - 'export { bar }', + 'let bar; export { bar }', ].join('\n'), options: [ 'always' ], errors: [ @@ -348,7 +348,7 @@ ruleTester.run('extensions', rule, { test({ code: [ 'export { foo } from "./foo.js"', - 'export { bar }', + 'let bar; export { bar }', ].join('\n'), options: [ 'never' ], errors: [ diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js index 92b3d9163e..569f3158a5 100644 --- a/tests/src/rules/named.js +++ b/tests/src/rules/named.js @@ -61,7 +61,7 @@ ruleTester.run('named', rule, { }), // regression tests - test({ code: 'export { foo as bar }'}), + test({ code: 'let foo; export { foo as bar }'}), // destructured exports test({ code: 'import { destructuredProp } from "./named-exports"' }), diff --git a/tests/src/rules/no-default-export.js b/tests/src/rules/no-default-export.js index 6440bfa895..5977ef19b2 100644 --- a/tests/src/rules/no-default-export.js +++ b/tests/src/rules/no-default-export.js @@ -29,7 +29,7 @@ ruleTester.run('no-default-export', rule, { `, }), test({ - code: `export { foo, bar }`, + code: `let foo, bar; export { foo, bar }`, }), test({ code: `export const { foo, bar } = item;`, @@ -42,6 +42,7 @@ ruleTester.run('no-default-export', rule, { }), test({ code: ` + let item; export const foo = item; export { item }; `, @@ -102,7 +103,7 @@ ruleTester.run('no-default-export', rule, { }], }), test({ - code: 'export { foo as default }', + code: 'let foo; export { foo as default }', errors: [{ ruleId: 'ExportNamedDeclaration', message: 'Do not alias `foo` as `default`. Just export `foo` itself ' + diff --git a/tests/src/rules/no-named-export.js b/tests/src/rules/no-named-export.js index d8748d3f6b..c56a135421 100644 --- a/tests/src/rules/no-named-export.js +++ b/tests/src/rules/no-named-export.js @@ -10,7 +10,7 @@ ruleTester.run('no-named-export', rule, { code: 'export default function bar() {};', }), test({ - code: 'export { foo as default }', + code: 'let foo; export { foo as default }', }), test({ code: 'export default from "foo.js"', @@ -82,7 +82,7 @@ ruleTester.run('no-named-export', rule, { }], }), test({ - code: `export { foo, bar }`, + code: `let foo, bar; export { foo, bar }`, errors: [{ ruleId: 'ExportNamedDeclaration', message: 'Named exports are not allowed.', @@ -111,6 +111,7 @@ ruleTester.run('no-named-export', rule, { }), test({ code: ` + let item; export const foo = item; export { item }; `, diff --git a/tests/src/rules/no-unresolved.js b/tests/src/rules/no-unresolved.js index 5b4f6ae53c..9db6e1a5c5 100644 --- a/tests/src/rules/no-unresolved.js +++ b/tests/src/rules/no-unresolved.js @@ -34,7 +34,7 @@ function runResolverTests(resolver) { rest({ code: 'export { foo } from "./bar"' }), rest({ code: 'export * from "./bar"' }), - rest({ code: 'export { foo }' }), + rest({ code: 'let foo; export { foo }' }), // stage 1 proposal for export symmetry, rest({ code: 'export * as bar from "./bar"' diff --git a/tests/src/rules/prefer-default-export.js b/tests/src/rules/prefer-default-export.js index 3a9145198d..3a89c6aa7f 100644 --- a/tests/src/rules/prefer-default-export.js +++ b/tests/src/rules/prefer-default-export.js @@ -28,6 +28,7 @@ ruleTester.run('prefer-default-export', rule, { }), test({ code: ` + let foo, bar; export { foo, bar }`, }), test({ @@ -44,11 +45,13 @@ ruleTester.run('prefer-default-export', rule, { }), test({ code: ` + let item; export const foo = item; export { item };`, }), test({ code: ` + let foo; export { foo as default }`, }), test({ diff --git a/tests/src/utils.js b/tests/src/utils.js index fe04d684e2..52e2d23cd5 100644 --- a/tests/src/utils.js +++ b/tests/src/utils.js @@ -43,8 +43,8 @@ export const SYNTAX_CASES = [ test({ code: 'const { x, y, ...z } = bar', parser: 'babel-eslint' }), // all the exports - test({ code: 'export { x }' }), - test({ code: 'export { x as y }' }), + test({ code: 'let x; export { x }' }), + test({ code: 'let x; export { x as y }' }), // not sure about these since they reference a file // test({ code: 'export { x } from "./y.js"'}), From 158cd8082b23a60264912fc6e557f168d3ec6b0f Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Sat, 2 Mar 2019 21:37:35 -0800 Subject: [PATCH 2/3] [Tests] use `resolve`, an actual dep, instead of `builtin-modules`, an erstwhile transitive dep --- tests/src/core/importType.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index abf9b95228..28c3c4ff70 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -90,16 +90,16 @@ describe('importType(name)', function () { }) it("should return 'external' for module from 'node_modules' with default config", function() { - expect(importType('builtin-modules', context)).to.equal('external') + expect(importType('resolve', context)).to.equal('external') }) it("should return 'internal' for module from 'node_modules' if 'node_modules' missed in 'external-module-folders'", function() { const foldersContext = testContext({ 'import/external-module-folders': [] }) - expect(importType('builtin-modules', foldersContext)).to.equal('internal') + expect(importType('resolve', foldersContext)).to.equal('internal') }) it("should return 'external' for module from 'node_modules' if 'node_modules' contained in 'external-module-folders'", function() { const foldersContext = testContext({ 'import/external-module-folders': ['node_modules'] }) - expect(importType('builtin-modules', foldersContext)).to.equal('external') + expect(importType('resolve', foldersContext)).to.equal('external') }) }) From e9544f83754bc74193f9caa6071c8c343b37c057 Mon Sep 17 00:00:00 2001 From: Guylian Cox Date: Thu, 6 Apr 2017 15:57:02 +0200 Subject: [PATCH 3/3] [Fix] Fix interpreting some external modules being interpreted as internal modules Fixes #793. - Add skipped test to expect scoped internal packages to be "internal" --- CHANGELOG.md | 5 +++++ package.json | 2 ++ src/core/importType.js | 6 +++++- tests/files/@importType/index.js | 1 + tests/files/order-redirect-scoped/module/package.json | 5 +++++ tests/files/order-redirect-scoped/other-module/file.js | 0 tests/files/order-redirect-scoped/package.json | 5 +++++ tests/files/order-redirect/module/package.json | 5 +++++ tests/files/order-redirect/other-module/file.js | 0 tests/files/order-redirect/package.json | 5 +++++ tests/src/core/importType.js | 10 ++++++++++ 11 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/files/@importType/index.js create mode 100644 tests/files/order-redirect-scoped/module/package.json create mode 100644 tests/files/order-redirect-scoped/other-module/file.js create mode 100644 tests/files/order-redirect-scoped/package.json create mode 100644 tests/files/order-redirect/module/package.json create mode 100644 tests/files/order-redirect/other-module/file.js create mode 100644 tests/files/order-redirect/package.json diff --git a/CHANGELOG.md b/CHANGELOG.md index d93839d76b..9427a79ae6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/package.json b/package.json index 79f52e5930..b114f9294d 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/core/importType.js b/src/core/importType.js index 89b162aad3..f2d6bda86e 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -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/ diff --git a/tests/files/@importType/index.js b/tests/files/@importType/index.js new file mode 100644 index 0000000000..bc4ffafc8c --- /dev/null +++ b/tests/files/@importType/index.js @@ -0,0 +1 @@ +/* for importType test, just needs to exist */ diff --git a/tests/files/order-redirect-scoped/module/package.json b/tests/files/order-redirect-scoped/module/package.json new file mode 100644 index 0000000000..6c4325e8e9 --- /dev/null +++ b/tests/files/order-redirect-scoped/module/package.json @@ -0,0 +1,5 @@ +{ + "name": "order-redirect-module", + "private": true, + "main": "../other-module/file.js" +} diff --git a/tests/files/order-redirect-scoped/other-module/file.js b/tests/files/order-redirect-scoped/other-module/file.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/files/order-redirect-scoped/package.json b/tests/files/order-redirect-scoped/package.json new file mode 100644 index 0000000000..8e1acfa874 --- /dev/null +++ b/tests/files/order-redirect-scoped/package.json @@ -0,0 +1,5 @@ +{ + "name": "@eslint/import-test-order-redirect-scoped", + "version": "1.0.0", + "private": true +} diff --git a/tests/files/order-redirect/module/package.json b/tests/files/order-redirect/module/package.json new file mode 100644 index 0000000000..6c4325e8e9 --- /dev/null +++ b/tests/files/order-redirect/module/package.json @@ -0,0 +1,5 @@ +{ + "name": "order-redirect-module", + "private": true, + "main": "../other-module/file.js" +} diff --git a/tests/files/order-redirect/other-module/file.js b/tests/files/order-redirect/other-module/file.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/files/order-redirect/package.json b/tests/files/order-redirect/package.json new file mode 100644 index 0000000000..1605a22035 --- /dev/null +++ b/tests/files/order-redirect/package.json @@ -0,0 +1,5 @@ +{ + "name": "eslint-import-test-order-redirect", + "version": "1.0.0", + "private": true +} diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index 28c3c4ff70..67de1d8a85 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -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')