Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(importType): Accept '@example' as internal #1493

Merged
merged 1 commit into from Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions CHANGELOG.md
Expand Up @@ -19,11 +19,12 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`default`]: make error message less confusing ([#1470], thanks [@golopot])
- Improve performance of `ExportMap.for` by only loading paths when necessary. ([#1519], thanks [@brendo])
- Support export of a merged TypeScript namespace declaration ([#1495], thanks [@benmunro])
- [`import/order`]: fix autofix to not move imports across fn calls ([#1253], thanks [@tihonove])
- [`order`]: fix autofix to not move imports across fn calls ([#1253], thanks [@tihonove])
- [`prefer-default-export`]: fix false positive with type export ([#1506], thanks [@golopot])
- [`extensions`]: Fix `ignorePackages` to produce errors ([#1521], thanks [@saschanaz])
- [`no-unused-modules`]: fix crash due to `export *` ([#1496], thanks [@Taranys])
- [`no-cycle`]: should not warn for Flow imports ([#1494], thanks [@maxmalov])
- [`order`]: fix `@someModule` considered as `unknown` instead of `internal` ([#1493], thanks [@aamulumi])

### Docs
- [`no-useless-path-segments`]: add docs for option `commonjs` ([#1507], thanks [@golopot])
Expand Down Expand Up @@ -146,7 +147,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-cycle`]: ignore Flow imports ([#1126], thanks [@gajus])
- fix Flow type imports ([#1106], thanks [@syymza])
- [`no-relative-parent-imports`]: resolve paths ([#1135], thanks [@chrislloyd])
- [`import/order`]: fix autofixer when using typescript-eslint-parser ([#1137], thanks [@justinanastos])
- [`order`]: fix autofixer when using typescript-eslint-parser ([#1137], thanks [@justinanastos])
- repeat fix from [#797] for [#717], in another place (thanks [@ljharb])

### Refactors
Expand Down Expand Up @@ -633,6 +634,7 @@ for info on changes for earlier releases.
[#1496]: https://github.com/benmosher/eslint-plugin-import/pull/1496
[#1495]: https://github.com/benmosher/eslint-plugin-import/pull/1495
[#1494]: https://github.com/benmosher/eslint-plugin-import/pull/1494
[#1493]: https://github.com/benmosher/eslint-plugin-import/pull/1493
[#1472]: https://github.com/benmosher/eslint-plugin-import/pull/1472
[#1470]: https://github.com/benmosher/eslint-plugin-import/pull/1470
[#1436]: https://github.com/benmosher/eslint-plugin-import/pull/1436
Expand Down Expand Up @@ -1029,3 +1031,4 @@ for info on changes for earlier releases.
[@maxmalov]: https://github.com/maxmalov
[@marcusdarmstrong]: https://github.com/marcusdarmstrong
[@Mairu]: https://github.com/Mairu
[@aamulumi]: https://github.com/aamulumi
2 changes: 1 addition & 1 deletion src/core/importType.js
Expand Up @@ -43,7 +43,7 @@ export function isExternalModuleMain(name, settings, path) {
return externalModuleMainRegExp.test(name) && isExternalPath(path, name, settings)
}

const scopedRegExp = /^@[^/]+\/[^/]+/
const scopedRegExp = /^@[^/]+\/?[^/]+/
ljharb marked this conversation as resolved.
Show resolved Hide resolved
function isScoped(name) {
return scopedRegExp.test(name)
}
Expand Down
3 changes: 2 additions & 1 deletion tests/src/core/importType.js
Expand Up @@ -30,6 +30,7 @@ describe('importType(name)', function () {
})

it("should return 'external' for scopes packages", function() {
expect(importType('@cycle/', context)).to.equal('external')
expect(importType('@cycle/core', context)).to.equal('external')
expect(importType('@cycle/dom', context)).to.equal('external')
expect(importType('@some-thing/something', context)).to.equal('external')
Expand All @@ -55,6 +56,7 @@ describe('importType(name)', function () {
it("should return 'internal' for internal modules that are referenced by aliases", function () {
const pathContext = testContext({ 'import/resolver': { node: { paths: [pathToTestFiles] } } })
expect(importType('@my-alias/fn', pathContext)).to.equal('internal')
expect(importType('@importType', pathContext)).to.equal('internal')
})

it("should return 'internal' for aliased internal modules that look like core modules (node resolver)", function () {
Expand Down Expand Up @@ -96,7 +98,6 @@ describe('importType(name)', function () {
})

it("should return 'unknown' for any unhandled cases", function() {
expect(importType('@malformed', context)).to.equal('unknown')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@malformed is not anymore an unhandled case. This is the purpose of this PR to accept @foo pattern.
The test has been replaced by this one.

expect(importType(' /malformed', context)).to.equal('unknown')
expect(importType(' foo', context)).to.equal('unknown')
})
Expand Down