extensions: add the checkTypeImports option

[phryneas]

Currently, the patterns

import type { Foo } from './foo';

export type { Foo } from './foo';

are ignored by the import/extensions rule.

This adds the checkTypeImports option to that rule, so these types of imports can be checked on an opt-in basis.

[ljharb]

Is this something that's more than purely style? At runtime, the types are erased, so does it matter what extensions are in the specifier?

[phryneas]

If you author a library, it is vital. If your types are missing the file extension (or have a .ts instead of a .js extension), consumers with the tsconfig setting moduleResolution: "node16" will not be able to use your library.

[ljharb]

That seems like a flaw in TS's moduleResolution then - it's not reasonable to expect your entire dep graph to update its specifier style just to make types work.

[phryneas]

Honestly: can we please not have this discussion about something we cannot change? In the end, import type is something the TS team invented, and this is how they decided it should work.

This is behaviour of TypeScript at least from version 4.7 (where node16 was introduced) to 5.2 (current beta), and even if TypeScript would suddenly change their mind, as library authors we need to write code that works with these TypeScript versions, too.

As you can see here, a library omitting the extension in their types will end up with an "Internal resolution error (2)" if consumed in ESM mode. here you can see that we fixed it. This is the PR that we did for that, and we would like to use this ESLint plugin to make sure that we keep our code compliant - isn't that pretty much the purpose of this plugin?

This PR just adds another option - it's not even on by default 😕

[ljharb]

That seems reasonable, but it's still worth pursuing a fix in TS so that the lint rule isn't needed in the long run - have you filed an issue there?

[phryneas]

There is ongoing discussion in many issues - I think their general argument is that it's required for all other imports to stay ESM-compatible, so it should for those, too.

There is movement to allow it together with allowImportingTsExtensions over in microsoft/TypeScript#54746, but I think it's more about .ts instead of .js, not about no extension at all. There is a lot of context in this gist by Andrew Branch, and I'm very sure they thought about that more than just twice.

Keep in mind, this is not about CJS packages being consumed by ESM, but about how the types of ESM packages consumed by ESM should look like, so they are striving for some kind of new standard here either way and "old ecusystem" won't be too affected by it, since that never shipped ESM in the first place.

[ljharb]

That's fine for first-party code, but requiring third-party packages to change how they do things so it works with a new system is exactly why native ESM has had such paltry adoption so far.

[phryneas]

🤷‍♂️ Please don't shoot the messenger. 😕

[ljharb]

Let's add a similar section under "never".

[phryneas]

See 6ac3d03

[phryneas]

Looking at those "opposite" tests, this will need to have #2813 merged in first so I can use ruleTesterWithTypeScriptImports in the tests - right now it does not resolve extensions correctly for .ts, so the { ts: 'always', tsx: 'always', js: 'always', jsx: 'always', checkTypeImports: true } line pretty much boils down to { checkTypeImports: true } for imports without extension.

I'll wait for that other PR and then rework the test for this one to include the correct parser.

[ljharb]

The ruleTesterWithTypeScriptImports part is pretty simple; I think you can pull that into this PR. If you leave it in a separate commit I can preserve authorship.

[phryneas]

I've added the comment to the doc you requested and some additional tests with ruleTesterWithTypeScriptImports.

As for code ownership - honestly, I don't care much about that, I have too many open PRs. Feel free to just edit away in this :)

[phryneas]

Seems like the CI errors are unrelated to this PR.