ChainSafe
Add linter rule to enforce proper type imports
Maybe we should add a linter rule that types should be imported with `import type` or `import {type` if possible.
Originally posted by @wemeetagain in https://github.com/ChainSafe/lodestar/issues/6384#issuecomment-1922123135
consistent-type-imports/ and consistent-type-exports/ rules do address this. Their usage is detailed on elsint blog.
Applying them on lodestar code base produces the following result:
✖ 3176 problems (3114 errors, 62 warnings)
3104 errors and 0 warnings potentially fixable with the `--fix` option.
Most of those errors can be fixed by eslint. Evenso the number of files impacted is huge, it might still be woth doing for the following reasons:
- do not depend on import side-effects for proper behavior
- allow tree-shaker to remove unneeded code (and reduce bundle sizes)
I'm in favor of adding these linter rules :+1:
- Also linking this other issue, https://github.com/ChainSafe/lodestar/issues/6011, since a nice endgame would be to have consistent rules across all our maintained repos.
it might still be woth doing for the following reasons:
Looking at some examples in our code https://github.com/ChainSafe/lodestar/blob/c4bf38576e0cba5880697de3a5ca4956162ede5e/packages/api/src/utils/server/genericJsonServer.ts#L1 It does not look like it make any difference as tsc is smart enough already to remove if from if only used as a type (I'd assume same applies for other bundlers)
Note that we already have import/no-extraneous-dependencies which prevents you from using fastify as a value in the api package as it is only a dev dependency.
I am not fully convinced that type imports are that useful in most cases, do we have an example in Lodestar were this makes a difference? As far as I can see the change from normal to type imports in https://github.com/ChainSafe/lodestar/pull/6384 does not matter
Not against adding these rules but we should have at least one specific case were a lint rule would have saved us before adding it.
