Auto-imports with NodeNext + isolatedModules missing type qualifier for decorated types

[benasher44]

Bug Report

🔎 Search Terms

• isolatedModules
• type
• import

🕗 Version & Regression Information

It's happening in 5.1.3, but it's surely been present since the 4.x version where "moduleResolution": "NodeNext" was supported.

💻 Code

See example below. The tsconfig should have the following configuration:

"isolatedModules": true,
"moduleResolution": "NodeNext",
"emitDecoratorMetadata": true,
"experimentalDecorators": true,

🙁 Actual behavior

When typing out the code on the last line and accepting the suggested import, you get the following import:

import { SomeType } from "apackage";

@a
const s: SomeType;

This import won't compile with this TS project configuration.

🙂 Expected behavior

Instead, you should get an import like this, since explicit type imports are required (compiler error) when isolatedModules is set to true, when moduleResolution is set to NodeNext and using decorators.

import type { SomeType } from "apackage";

@a
const s: SomeType;

Today, the workaround is to use an eslint rule to help auto-fix these issues.

[andrewbranch]

This import won't compile with this TS project configuration.

The settings you specified definitely have no inherent problem with the code example you gave. That would be an error under importsNotUsedAsValues: error, but auto-imports does account for that if it’s working properly. It sounds like maybe you read an eslint error from the rule you mentioned as a TypeScript error 😅. I could be wrong, but you’ll need to provide a real repro here.

[fatcerberus]

Related: #54664

[benasher44]

I can reproduce the error by running tsc. I'll figure out a real repro.

[fatcerberus]

@andrewbranch Wait, this is not an error under isolatedModules? That doesn’t seem right… my understanding was, isolatedModules is supposed to error if correct emit requires type info from another file—which it does here, in order to know it has to erase the import.

[andrewbranch]

It doesn’t require type info, because SomeType is never used in a JS-emitting position, which single-file transpilers universally know. It doesn’t really matter for emit (under these settings) whether SomeType is in fact a type or a value or both, because it’s only ever used in a type annotation which is unambiguously erased, so the import will be elided by default.

[benasher44]

Okay sorry for the confusion. The issue is specifically one for decorators, not everything. I was clouded by the all-or-nothing approach of figuring out a way to get this auto-fixed, by pulling in eslint to just be consistent about all such imports (any that could do import type, not just those where a decorator is involved).

Here's the exact diagnostic that is emitted, if you don't import type such a type:

 TS1272: A type referenced in a decorated signature must be imported with 'import type' or a namespace import when 'isolatedModules' and 'emitDecoratorMetadata' are enabled.

[benasher44]

This only seems to be an error with NodeNext (or maybe NodeNext + esm) and isolatedModules, and TS server's import suggestion chooses import instead of import type

[benasher44]

I tried making a codesandbox, but I wasn't able to reproduce it there. Event in my local project, I'm staring at two very similar decorated class properties, and TS is only emitting this error for one of them 🤔

[andrewbranch]

Is the one that’s not erroring referencing a class, and one that’s erroring referencing an interface/type? 😄

I recall adding that error for decorator metadata, and I probably didn’t adjust auto-imports accordingly. It is a bug, but the fact that it’s decorator metadata makes it pretty edge-casey in my opinion. If it were specific to experimentalDecorators, I’d say won’t fix, but I’m pretty sure it will repro with real decorators.

[benasher44]

👍 yep I think that's why! Okay cool; we use sequelize-typescript, so we get a lot of mileage out of decorators, which is why we saw this everywhere 😅