Proposal: Always allow type-only imports to reference .ts extensions

[andrewbranch]

Currently, it’s an error to have a type-only import from a .ts extension unless allowImportingTsExtensions is enabled:

import type { JustAType } from "./justTypes.ts";
//                             ^^^^^^^^^^^^^^^^
// Error: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.

I’m pretty sure this was an oversight in #51669/#52230 where I first introduced allowImportingTsExtensions. Whether the flag is set or not, type-only imports are allowed to import from a .d.ts extension, so it makes sense to allow it for .ts extensions too. What I intended to implement was:

1. Importing from a TS file extension always resolves
2. Importing from a TS file extension is allowed as long as it doesn’t emit to JS
3. allowImportingTsExtensions removes the “as long as it doesn’t emit to JS” exception from (2)

This was brought up as a complaint in #51876.

[robpalme]

Is there already an equivalent test case of the 5 assertions below for the case where a.ts is a.d.ts?

[andrewbranch]

I think so, but I'll add one here so it can be compared easily

[sheetalkamat]

Interesting i would have expected above two to be not errors per your description since its not in emitted?

[andrewbranch]

It’s a bit of a gray area, but I decided to treat this as if verbatimModuleSyntax is enabled. Depending on settings, this import could get preserved as import {} from "./a.ts", whereas import type is unambiguously elided. It feels better to rely on the strong, unambiguous signal that the import will always be elided.

[jakebailey]

I feel like this is effectively always a bad idea; if you ever want to update an import to include something that isn't a type, you now have to rewrite the import path, when you could have just written the import the correct way the first time.

Am I missing a use-case here? I don't find the comment in #51876 (comment) very compelling and it seems to be wanting to resolve .ts for non-type reasons... Ignoring all of our arguments as to why we want imports written as they are to be emitted, not what necessarily is on disk at the time.

I guess if this is the intended behavior then it seems fine, but I do feel like nobody should be doing this, and everyone who does is just going to be confused and complain that TS is doing something wrong (when, we aren't, as is usual for every module resolution "bug").

[andrewbranch]

I don’t find the “you have to rewrite the module specifier to include a non-type” argument very compelling, because the main thing you have to do to reference the things you import in value positions is remove the type modifier from import type. Doing that is a significant change and I think people understand that.

I personally find it weird to see an import like

import type { SomeType } from "./types.js"

Many people always find .js extensions in TS files weird, but I’m satisfied with the explanation that you write the thing that’s going to work at runtime. But that explanation just doesn’t work for something that we know never has a runtime representation. A type-only import is more like a triple-slash reference in that way, and we notably don’t make you write

/// <reference path="fourslash.js" />

[jakebailey]

I don’t find the “you have to rewrite the module specifier to include a non-type” argument very compelling, because the main thing you have to do to reference the things you import in value positions is remove the type modifier from import type. Doing that is a significant change and I think people understand that.

Concretely, the transition I'm referring to is when one has the import:

import type { Blah } from "./something.ts"

If they previously only used Blah as a type, then this is legal, but the thing I'm describing is when someone needs to add another import from that same file, or just wants to instantiate Blah (maybe it's a class).

My understanding is that auto-import or quickfix will attempt to reuse this import declaration, meaning they'll just drop the type modifier, leaving the import in an invalid state because you can't non-type import from a .ts file.

If auto-import/quickfix don't do this, then they'll instead introduce duplicate import declarations, in which case future imports/fixes will have to ambiguously distribute over multiple imports, which sounds weird.

I personally find it weird to see an import like

import type { SomeType } from "./types.js"

With verbatimModuleSyntax, that is what we force people to write, though. ts-eslint now has a rule for (not enabled by default, yet) to avoid accidentally emitting the side effect import {} from "./types.js".

[andrewbranch]

My understanding is that auto-import or quickfix will attempt to reuse this import declaration, meaning they'll just drop the type modifier, leaving the import in an invalid state because you can't non-type import from a .ts file.

Yeah, your message made me realize that auto-imports needs to handle this for you if it doesn’t already. Auto-imports is never supposed to result in an error. That should be an easy fix.

With verbatimModuleSyntax, that is what we force people to write, though. ts-eslint now has a rule for (not enabled by default, yet) to avoid accidentally emitting the side effect import {} from "./types.js".

The part that I meant was weird about that was the .js extension in combination with import type. Because import type makes it purely a TS-internal resolution, but the .js extension is purely for the benefit of a supposed runtime. The import type bit is non-negotiable, but the need to write .js can be lifted.

[andrewbranch]

After doing some development for arethetypeswrong in JSDoc, I’m even more convinced that this is desirable. Some JSDoc-annotated JS files reference a types file that’s in some scripts that get executed with tsx, so no JS file for those ever exists. And yet, in my JSODoc type tags, I’m forced to reference it like

/** @type {import("./types.js").Foo} */

instead of

/** @type {import("./types.ts").Foo} */

And in this case, I definitely should not enable allowImportingTsExtensions, because I’m running these JS files directly in Node, which will crash if I do a runtime import of a .ts file. I can’t make an argument with a straight face that module specifier reference rules are motivated by emit constraints when silly counterexamples like this exist.

[jakebailey]

It does seem like the upsides outweigh the downsides given the above, though I do think we need to file an issue for or fix soon the fact that auto-imports may misbehave when this is allowed. At least nobody would have written this so far.

Is this still good to go in pre-RC or does it need to wait?

[andrewbranch]

I think this can go in pre-RC since it only removes an error and otherwise doesn’t change any behavior. I can fix the auto-import behavior as part of this PR.

[andrewbranch]

@jakebailey auto-imports changes are done; thanks for the reminder 👍

[jakebailey]

Auto-imports look good to me, thanks!