-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[Tests] extensions
: add cases to verify @/configs/chart
is treated as a package
#1925
Conversation
…d as a package See import-js#1851.
@/configs/chart
being treated as a package.
tests/src/rules/extensions.js
Outdated
@@ -367,6 +368,11 @@ ruleTester.run('extensions', rule, { | |||
line: 4, | |||
column: 31, | |||
}, | |||
{ | |||
message: 'Missing file extension for "@/configs/chart"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again tho, this should never have a warning when ignorePackages
is set, because it's a bare idenfier - a package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the scoped packages doc, @somescope/somepackagename
is a scoped package, whereas @/foo/bar
is not. Publishing @/foo/bar
as an unscoped package is also not allowed, so it's neither a scoped package nor an unscoped package, therefore not a package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it isn't "a scoped package", but since it doesn't start with .
or /
, it is a "bare identifier", which is always "a package" - packages aren't just "publishable packages", they're "anything with a bare identifier".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Circling back on this - @
is a package here (just not a publishable one), so I think this should be ignored when ignorePackages
is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased this to only include updated test cases, since this is already working as intended.
tests/src/rules/extensions.js
Outdated
@@ -367,6 +368,11 @@ ruleTester.run('extensions', rule, { | |||
line: 4, | |||
column: 31, | |||
}, | |||
{ | |||
message: 'Missing file extension for "@/configs/chart"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Circling back on this - @
is a package here (just not a publishable one), so I think this should be ignored when ignorePackages
is enabled.
@/configs/chart
being treated as a package.extensions
: add cases to verify @/configs/chart
is treated as a package
Fixes #1851
Before this fix,
'import/extensions': ['warn', 'ignorePackages']
wouldn't warnimport chart from '@/configs/chart'
, this is an improvement over PR #1854.