eslint-plugin-import: [import/no-extraneous-dependencies] Problems when only using types
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.
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 3
- Comments: 42 (22 by maintainers)
Commits related to this issue
- Work around a bug in import/no-extraneous-dependencies https://github.com/benmosher/eslint-plugin-import/issues/1618 — committed to gatesolve/gatesolve-ui by tuukka 4 years ago
Is there a way to suppress this error when using new TS syntax
import type ...
? Or any other workaround apart from naming all the modules inimport/internal-regex
?causes
ESLint: 'aws-lambda' should be listed in the project's dependencies. Run 'npm i -S aws-lambda' to add it(import/no-extraneous-dependencies)
in my devDependencies
Created the PR and added tests.
Funnily enough this plugin already handles type-only imports for Flow (
import type
), which is now coming to TS (https://devblogs.microsoft.com/typescript/announcing-typescript-3-8-beta/#type-only-imports-exports). Type imports are ignored in many places in this plugin, includingno-extraneous-dependencies
: https://github.com/benmosher/eslint-plugin-import/blob/3b04d5fab6c095e7f0f99488665d90e285872271/src/rules/no-extraneous-dependencies.js#L102-L105Would 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:
https://github.com/typescript-eslint/typescript-eslint/issues/1436 https://github.com/typescript-eslint/typescript-eslint/pull/1465 https://github.com/babel/babel/issues/10981
I think we should wait, instead of complicating and adding specific
@types
logic in here.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? 😄
@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 it should not affect non-typescript setups as it was only applied to the typescript config. If you don’t have
extends: ['plugin:import/typescript']
in your eslint config, then that change doesn’t apply.I have also tried a minimal repro with your issue and it’s working fine for me:
Have both
ioredis
and@types/ioredis
installed globally andnode index.js
runs fine.Will look into the original issue now. Thanks @ljharb for pinging.
In line type imports do not work.
@ljharb oh I realize now I misunderstood this issue. I thought there was a false positive for
<something> should be listed in the project's dependencies. (...)
and the fix took care of it. 🤦 Usingimport type
and adding the location of the@types
packages like below worked fine.@vamcs in that case, please file a new issue with the error, the code being linted, and the relevant eslint/plugin/typescript parser versions 😃
(make sure you’re on the latest typescript parser first)
Looking a bit closer to this issue I found that the
node
at that point doesn’t have aimportKind
, but it is in it’sparent
@ljharb could it be that in the rule it would need
to be compatible with the typescript
import type
?Please file a PR with failing test cases; that makes things much easier to fix.
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.
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
{ path, found }
object)?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
import/external-module-folders
setting which treats them equally.I can draft a PR tomorrow and discuss there.
@thewilkybarkid @ljharb I believe the current behaviour is correct and this is a bug.
Using the original example:
In both versions it resolves to:
But it
2.18.2
it was being marked asinternal
.#1526 fixed that, so it is now correctly marked as
external
.By being
external
it’s now subject to the check ofno-extraneous-dependencies
according to https://github.com/benmosher/eslint-plugin-import/blob/f790737c07f39058592b7eb8b6cda3de72c239b9/src/rules/no-extraneous-dependencies.js#L118-L120 - which didn’t happen before at all.And because the package name is computed as
jsonld
(from the import path) rather than@types/jsonld
, it is never found in the package deps.Changing the import to:
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
jsonld
and@types/jsonld
if the resolved path contains@types
? I know it adds some knowledge of the TS structure, butExportMap
already contains TS logic as well.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
eslint-import-resolver-babel-module
as a resolver.cc @joaovieira do you have time to take a look at this? I’d prefer not to revert your PR.