add consistent type import eslint rule

[aruniverse]

• Add @typescript-eslint/consistent-type-imports to recommended eslint plugins

  • had to disable no-duplicate-imports to get this to work

• Add lint:fix npm scripts

• Run new rule across entire monorepo

[kabentley]

I'm sorry, I just don't understand the benefit of this rule. Can you please explain?

[aruniverse]

I'm sorry, I just don't understand the benefit of this rule. Can you please explain?

Hey, sorry it took so long to get back with an answer; this was super low on the 3.0 priority list.

I primarily had 2 reasons for wanting to introduce this change:

• Determining dependencies for packages

  • Most, if not all, of us, have had a really hard time determining iTwin.js dependencies when writing a package. Due to the nature of all the statics and side effects in our code, we can't use direct dependencies in most cases. We ended up going with a dev + peer dep approach, but no one really knew which dependencies should be listed as a peer, so we ended up just listing all the same dev deps as peers. Often we're just only importing types or interfaces from a package so they should technically only be a devDep. With this change, you should be able to easily grep for use of a package, and look at the imports at the top of the file for you to quickly determine if the package should be a peer dep or not.

• Extension API

  • During the extension hackathon, when trying to convert existing functionality into an extension and determining what to expose from our extension framework, we spent more time than I would've liked determining whether we needed a real import (value) or not. This should help with that as well, if we could quickly look at the imports to determine if its a type or value.

At the end of the day, types/interfaces are "syntactic sugar" in typescript that won't get emitted in the transpiled javascript code we deliver. This should have no impact on consumers besides being a utility to help developers. I think it'll be worth it and has already proven to be helpful in determining deps of the vcr and client repos.

Additional reading on type-only imports/exports if you're interested:
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
https://stackoverflow.com/questions/61412000/do-i-need-to-use-the-import-type-feature-of-typescript-3-8-if-all-of-my-import

[wgoehrig]

@aruniverse I have somewhat mixed feelings on this rule...

I'm definitely a huge fan of the type modifier in imports, since I think it makes it much easier for a developer to picture and reason about the JavaScript that the TypeScript compiler will actually emit, and I do think using it consistently would be pretty helpful.

But two things are concerning to me here:

• I see imports with both values and types being broken into two separate import statements (each on their own line). Personally, I think we have too many lines of imports everywhere as is... Could this maybe wait until we're on 4.5 (at which point we could use the type modifiers on import names) instead?
• I'm pretty fond of VS Code automatically adding imports for me - is there a way to have it use the correct modifiers too, or will auto-imports start creating lint failures? I can see that getting really annoying.

[aruniverse]

Good points @wgoehrig !

Re 1: Agreed, there are some files that are already larger than they ought to be, and this def doesn't help. A little bit more uglier and effort is needed to determine if a pkg can be a devDep imo (I'm prob very lazy so that doesn't help), but I can see the benefit of saving lines, and it's not that bad.

Re 2: From what ive seen vscode won't automatically add the type annotation. Agreed this can be very annoying if the linter fails your pr because of this; its why I added the lint:fix script.

I think we'll need to wait on this, typescript-eslint/typescript-eslint#4338

I'm not opposed to leaving this PR as a draft until we find a perfect solution.

[paulius-valiunas]

@aruniverse I was wondering why you added the lint:fix script... I don't think you need it, vscode should do it automatically when you save a file. The only potential problem is that you had to disable no-duplicate-imports so we might end up with several type imports from the same module.

[gajus]

You can now achieve this with canonical/prefer-inline-type-import.