eslint-plugin-import: `no-unused-modules` does not work properly with flow types exports

This PR introduces support for Flow type exports https://github.com/benmosher/eslint-plugin-import/pull/1542

However, it only takes into account the identifier import style https://github.com/benmosher/eslint-plugin-import/pull/1542/files#diff-48b646acd6be2c915b501dc667163bd4

And it does not work for the declaration import style. Example Module a.js:

export type A = 'string'

Module b.js:

import type { A } from 'a.js'`

Will result in an error in module A:

ESLint: exported declaration 'A' not used within other modules(import/no-unused-modules)

Note: It seems to work fine for me if I use identifier import style

import { type A } from 'a.js'`

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 29 (27 by maintainers)

Commits related to this issue

[Fix] `no-unused-modules`: ignore flow type imports Fixes #1564. — committed to nicolashenry/eslint-plugin-import by nicolashenry 4 years ago

Most upvoted comments

On the other hand, it isn’t clear to me why import { type A } from '...' works since imports with only flow types are skipped too here

That’s because in this case specifier.importKind does not equal type. It equals value instead, hence it isn’t catched.

Meanwhile i reverted the changes of #1493 locally but kept the tests, trying to make them still pass but with different changes on the source files. Only one is left so far, so if I am lucky, I will be able to provide a solution soon.

rfermann on Dec 12, 2019

Thanks for adding, @ljharb.

@yamov i‘m not familiar with flow at all. If you could provide all different ways of imports and exports particularly used by flow, I would be willing to (try to) add support for these.

rfermann on Dec 10, 2019

@ljharb the next week is pretty busy for me, but I guess I’ll have some free time after that for a fix and test coverage

maxmalov on Dec 26, 2019

@ljharb I created https://github.com/benmosher/eslint-plugin-import/pull/1906 to ignore Flow types again. I think babel-eslint parser causes an issue in module load ordering when flow type imports are used but I don’t think that I can fix it (unless you have an idea on how to do it).

nicolashenry on Sep 22, 2020

@Hypnosphi It seems to be related to the loading order, if I rename a.js to b.js, b.js to a.js and I change from './a' to from './b' in your repository I no longer have the error… I am not sure about how to fix this for now.

nicolashenry on Sep 21, 2020

@nicolashenry sure: https://github.com/Hypnosphi/import-flow-repro

Hypnosphi on Sep 21, 2020

I will take a look when I have more time but it seems that I may introduced wrongly TypeAlias and InterfaceDeclaration by babel-eslint as Typescript types when it seems to be Flow types. So some typescript tests which existed before my changes were probably relying wrongly on babel-eslint parser and I tried to make it compatible with it too but it was probably a bad idea unless some people are using babel-eslint parser with Typescript source code instead of @typescript-eslint/parser or typescript-eslint-parser.

I will try to change typescript tests to use typescript parsers only or maybe try to do a proper fix for Flow type imports but I am not a Flow user so I will may have some difficulties to do it.

nicolashenry on Sep 21, 2020

Seems fixed in 2.20

Hypnosphi on Jan 30, 2020

Also, instead of removing only type imports from the dependency graph we can add some meta information to them, e.g typesOnly: true. This can be used in the cycle calculation phase later, and hopefully will fix no-unused-module rule. Since types module will exist in the dependency tree, but will be treated differently only in no-cycle rule.

maxmalov on Dec 13, 2019