eslint: Enable consistent-type-imports

[auscompgeek]

Revives #872.

It might be a good idea to enable import/no-duplicates first though.

https://typescript-eslint.io/rules/consistent-type-imports/

🥞

• tsconfig: Enable isolatedModules #1829

Checklist

• I have added tests
• I have updated the docs and cheatsheet
• I have not broken the cheatsheet

[pokey]

Had one question

Also, re running deduplicate, do we not want to run deduplicate after this one rather than before, due to interaction with import-js/eslint-plugin-import#334. Not totally sure tbh 🤔

[pokey]

It's possible to do this in one line

Suggested change

	import type { CheatsheetInfo } from "@cursorless/cheatsheet";
	import { CheatsheetPage } from "@cursorless/cheatsheet";
	import { type CheatsheetInfo, CheatsheetPage } from "@cursorless/cheatsheet";

Is there a way to get it to do that?

[pokey]

Huh looks like there are 4 different lint rules / options related to this question:

• https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports
• https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md
• https://typescript-eslint.io/rules/no-import-type-side-effects/
• https://typescript-eslint.io/rules/consistent-type-imports/#fixstyle

I'd be tempted to have one PR that gets us to the configuration we're after in one go, rather than multiple PRs that touch all of our imports, given that I'd imagine they could interact in interesting ways

[pokey]

@auscompgeek maybe let's aim to discuss this inline type thing at a meet-up? to discuss Plan to discuss at meet-up

[pokey]

After discussion, we want to:

1. enable "@typescript-eslint/consistent-type-imports": "error"
2. enable --verbatimModuleSyntax TS flag
3. enable "import/consistent-type-specifier-style": ["error", "prefer-top-level"]
4. enable "import/no-duplicates": "error" (note we don't need prefer-inline because we don't use inline)

The reasoning is that we want import type to be like its own kind of statement that we know always gets dropped in the output Javascript, and in addition, if we were to have a rule where import {type foo} => import type {foo} only when there are no non-type imports in the statement, it would feel a bit inconsistent that it doesn't happen when there are non-type imports. The proposed rule is simpler: types always get imported in their own statement, and never have side effects

Each step should be its own PR, and they should be in the above order. Note that 1) must come before 2) because without 2), 1) should have no side effects, and then the effects of 2) should become more clear once 1) is in

Note that we don't need no-import-type-side-effects lint rule because 2 and 3 won't allow us to have inline type anywhere

[pokey]

We want to test the following cases:

• you use something as a type but not runtime and it's currently imported as runtime
• you use something as a type that is not yet imported
• you use something as runtime that is currently imported as a type

For each of the above cases, we want to test:

• Auto-fix
• Organize imports
• Auto-import

[auscompgeek]

I still never got a chance to test that matrix, but up until TypeScript 5.3 whether auto-import used import type depends on the tsconfig. TypeScript 5.3 adds an IDE setting to prefer auto-importing with import type https://devblogs.microsoft.com/typescript/announcing-typescript-5-3/#settings-to-prefer-type-auto-imports

The presence of this lint rule will obviously not affect auto-import behaviour since eslint doesn't change the tsconfig. The auto-import behaviour probably isn't too big of an issue anyway since we all have eslint autofix on save, I believe.

[pokey]

Note from meet-up:

• For ease of git-fu, just open a new PR with contents of first commit
• Then test the matrix in Enforce type in front of type-only imports #2618

[AndreasArvidsson]

Replaced by #2619