-
-
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
[import/no-extraneous-dependencies] Problems when only using types #1618
Comments
Similar to #1615? |
Went through the commits since 2.18.2 on |
Can confirm this first appears in 2.20.0, and #1615 appears in 2.19.1 (maybe 2.19.0, but that's broken). |
Seems that #1526 is cause, reverting the change causes it to pass. |
To clarify, I have
(The latter being impossible as it doesn't exist.) My config has: settings: {
'import/resolver': {
typescript: { alwaysTryTypes: true },
},
}, Without that the errors are:
|
The intention of the no-unresolved rule, however, is to never import something that doesn't exist - iow, why is |
They are both installed. |
Just tried moving |
hmm, which typescript resolver are you using? i believe the unscoped one is deprecated. |
2.0.0 (.eslintrc.js, package.json). |
Unless I'm missing some new setting, I believe this (#1526) may be affecting the For example, I can add an import for Maybe I'm just not doing something right? I'm using v2.20.1. [EDIT] Yep, if I revert back to 2.18.2 then I correctly get the error: |
cc @joaovieira do you have time to take a look at this? I'd prefer not to revert your PR. |
@dlong500 it should not affect non-typescript setups as it was only applied to the typescript config. If you don't have I have also tried a minimal repro with your issue and it's working fine for me: // .eslintrc
module.exports = {
env: {
node: true
},
extends: [
'eslint:recommended',
'plugin:import/recommended',
],
plugins: ['import'],
settings: {
'import/resolver': {
node: {
paths: ['/Users/joaovieira/.nvm/versions/node/v10.16.3/lib/node_modules']
}
}
},
rules: {
'import/no-extraneous-dependencies': 1
}
}; // package.json
"dependencies": {
"eslint": "^6.8.0",
"eslint-plugin-import": "^2.20.1"
} Have both Will look into the original issue now. Thanks @ljharb for pinging. |
@joaovieira you'll also need the typescript resolver. |
@joaovieira I definitely don't have any typescript rules in my eslint config, so something else must be coming into play. I figured it must have something to do with vscode's Automatic Type Acquisition related to intellisense. I'm also using vscode's nightly (insider) build, so I don't know if that might affect things too. I'll try to continue testing on my end to see if I can pinpoint anything useful. Sorry, I don't mean to hijack the thread if this isn't directly related to the OP--just not sure where else to post as there is definitely something different between v2.18.2 and v2.20.1. |
@dlong500 if you're having issues in vscode, verify on the command line - if it works there, it's a vscode bug only. |
Sorry to be a pain, but the issue persists with running eslint from the command line (same difference between v2.18.2 working and v2.20.1 not working). I will try to come up with a test case repo if I can get a bit of time. One thing to point out is that I'm using |
@thewilkybarkid @ljharb I believe the current behaviour is correct and this is a bug. Using the original example: import { Context as JsonLdContext } from 'jsonld/jsonld-spec'; In both versions it resolves to: .../node_modules/@types/jsonld/jsonld-spec.d.ts But it #1526 fixed that, so it is now correctly marked as By being And because the package name is computed as Changing the import to: import { Context as JsonLdContext } from '@types/jsonld/jsonld-spec'; Would treat it as a scoped module and pass the eslint check, but doesn't make the TS compiler happy. @ljharb what would you think of trying both |
Is there any way we could mark it as a TS type (and thus eligible for the dual check) only if the TS resolver marks it as external? ie, so only these cases would get treated as such. |
The resolver knows it's a "definitely typed" module and could add any flag at that point: https://github.com/alexgorbatchev/eslint-import-resolver-typescript/blob/ef5c33d3bc561e1ef34539b752b40de1685e5454/src/index.ts#L86 but I assume the resolvers have a well-defined contract (i.e. return a Though it's actually the import plugin that checks if it's external, but at that point it can't distinguish if it's a "definitely typed" or normal module as its driven by the I can draft a PR tomorrow and discuss there. |
A |
So, I've discovered my issue only occurs on Windows (and only with the very latest v2.20.1). The same repo on Linux works fine. I think it is related to #1643. I did create a minimal testing repo here (https://github.com/dlong500/eslint-import-extraneous-issue-simple) in case it is helpful. I'm seeing the same issue show up even with the standard node resolver (no custom resolvers). Sorry if all this is unrelated to the the actual issue in this thread. |
Funnily enough this plugin already handles type-only imports for Flow ( Would be good to treat them the same way (between Flow and TS), but we have to wait for that to be available in the TS AST. That's already on the way: typescript-eslint/typescript-eslint#1436 I think we should wait, instead of complicating and adding specific |
I see the same error for type-fest. Using the following as workaround. {
...,
settings: {
'import/internal-regex': '^type\\-fest$',
},
...
} |
Is there a way to suppress this error when using new TS syntax
causes in my devDependencies
|
Typescript 3.8 is out, and looks like the situation of using |
Please file a PR with failing test cases; that makes things much easier to fix. |
Looking a bit closer to this issue I found that the
@ljharb could it be that in the rule it would need
to be compatible with the typescript |
That very well might be exactly what's needed (but probably would be needed in dozens of places). Want to make a PR with some tests? :-D |
Created the PR and added tests. |
❓ is this fix released already? |
@vamcs yes - if you click on the commit link above, you'll see it's included in v2.22.0. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hello! I think this is the same issue, but shout if it's separate.. I'm still getting this issue on version See this GitHub Action build: https://github.com/samtgarson/hooks/runs/1862888533?check_suite_focus=true Specifically I'm seeing the plugin give errors for Interestingly, it's ignoring the |
@samtgarson the As for the imports themselves - can you use |
It's also erroring in
This issue is specifically about not using
I'll pull master and check. If not, does this warrant reopening the issue? |
@ljharb can confirm this issue exists on my repo when using I'm not sure the relevance of this but after a quick dig, it appears the |
If that's the case, then it's coming from the TS eslint parser, not from this plugin - can you file an issue there and link it here? |
In line type imports do not work. // works
import type { Foo } from "types-package";
// doesn't work yet
import { type Foo } from "types-package". |
We've started to see failures when moving from 2.18.2 to 2.20.0 regarding importing only types from
devDependencies
in TypeScript source code. Previously the rule wasn't triggering for these (which I would expect), but it now is.Refs libero/article-store#174.
The text was updated successfully, but these errors were encountered: