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.

Refs https://github.com/libero/article-store/pull/174.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 3
  • Comments: 42 (22 by maintainers)

Commits related to this issue

Most upvoted comments

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 in import/internal-regex?

import type { APIGatewayEvent, APIGatewayEventRequestContext } from 'aws-lambda';

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

"@types/aws-lambda": "8.10.40",

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, including no-extraneous-dependencies: https://github.com/benmosher/eslint-plugin-import/blob/3b04d5fab6c095e7f0f99488665d90e285872271/src/rules/no-extraneous-dependencies.js#L102-L105

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:

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:

// .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"
}
Screen Shot 2020-02-09 at 21 26 35

Have both ioredis and @types/ioredis installed globally and node index.js runs fine.


Will look into the original issue now. Thanks @ljharb for pinging.

In line type imports do not work.

// works
import type { Foo } from "types-package";

// doesn't work yet
import { type Foo } from "types-package".

@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. 🤦 Using import type and adding the location of the @types packages like below worked fine.

"node": {
	"extensions": [".js", ".ts"],
	"paths": ["node_modules/", "node_modules/@types"]
},

@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 a importKind, but it is in it’s parent

{
  type: 'Literal',
  raw: "'type-fest'",
  value: 'type-fest',
  range: [ 33, 44 ],
  loc: { start: { line: 1, column: 33 }, end: { line: 1, column: 44 } },
  parent: {
    type: 'ImportDeclaration',
    source: [Circular *1],
    specifiers: [ [Object] ],
    importKind: 'type',
    range: [ 0, 45 ],
    loc: { start: [Object], end: [Object] },
    parent: {
      type: 'Program',
      body: [Array],
      sourceType: 'module',
      range: [Array],
      loc: [Object],
      tokens: [Array],
      comments: [],
      parent: null
    }
  }
}

@ljharb could it be that in the rule it would need

// Do not report when importing types 
 if (node.importKind === 'type' || node.parent.importKind === 'type') { 
   return 
 } 

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:

import { Context as JsonLdContext } from 'jsonld/jsonld-spec';

In both versions it resolves to:

.../node_modules/@types/jsonld/jsonld-spec.d.ts

But it 2.18.2 it was being marked as internal.

#1526 fixed that, so it is now correctly marked as external.

By being external it’s now subject to the check of no-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:

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 jsonld and @types/jsonld if the resolved path contains @types? I know it adds some knowledge of the TS structure, but ExportMap already contains TS logic as well.

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