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

Open
wants to merge 1 commit into
base: master
from

Conversation

@AamuLumi
Copy link

commented Oct 1, 2019

Hi !

Fix #1379

The bug was related to the internal regex which didn't accept a module with only one word.
This is the case in Typescript when you use the tconfig paths to accept the import of @myModule in replacement of /src/someFolder/myModule.

Thanks for developing this library!

Fixes #1379
@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.08%) to 96.47% when pulling ea6019c on AamuLumi:master into e62011f on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+1.0%) to 96.32% when pulling ff8e516 on AamuLumi:master into 5e143b2 on benmosher:master.

3 similar comments
@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+1.0%) to 96.32% when pulling ff8e516 on AamuLumi:master into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+1.0%) to 96.32% when pulling ff8e516 on AamuLumi:master into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+1.0%) to 96.32% when pulling ff8e516 on AamuLumi:master into 5e143b2 on benmosher:master.

@ljharb ljharb force-pushed the AamuLumi:master branch from ff8e516 to 5264464 Oct 8, 2019
@@ -43,7 +43,7 @@ export function isExternalModuleMain(name, settings, path) {
return externalModuleMainRegExp.test(name) && isExternalPath(path, name, settings)
}

const scopedRegExp = /^@[^/]+\/[^/]+/
const scopedRegExp = /^@[^/]+\/?[^/]+/

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 8, 2019

Collaborator

hmm, this also seems to allow @foo/ - can you add an explicit test for that?

This comment has been minimized.

Copy link
@AamuLumi

AamuLumi Oct 9, 2019

Author

When I tested, the @foo/ case is recognized as external.
Is it correct, or must I change that?

@ljharb ljharb force-pushed the AamuLumi:master branch from db51e3b to ea6019c Oct 16, 2019
@@ -96,7 +98,6 @@ describe('importType(name)', function () {
})

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

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 16, 2019

Collaborator

why does this need to be removed?

This comment has been minimized.

Copy link
@AamuLumi

AamuLumi Oct 16, 2019

Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.