emitDecoratorMetadata causes runtime errors by referencing type-only imports with namespaces

[liorcode]

Bug Report

Related to #37672 and #39337 - seems this bug still happens if class is wrapped by a namespace.

🔎 Search Terms

emitDecoratorMetadata, type-only imports, decorators

🕗 Version & Regression Information

Typescript 4.1.3, Typescript 4.2.0 beta

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about emitDecoratorMetadata and type-only imports

⏯ Playground Link

Workbench Repro

💻 Code

// @experimentalDecorators
// @emitDecoratorMetadata
// @strict: false


// @filename: services.ts
export namespace Services {
  export class Service {}
}

// @filename: index.ts
import type { Services } from './services';

declare const decorator: any;
export class Main {
  @decorator()
  field: Services.Service;
}

🙁 Actual behavior

Typescript will reference "Services.Service" even though it's not imported (only the type was). This will crash in runtime:

__decorate([
decorator(),
__metadata("design:type", Services.Service)
], Main.prototype, "field", void 0);

🙂 Expected behavior

Should not reference non imported classes:

__decorate([
decorator(),
__metadata("design:type", Object)
], Main.prototype, "field", void 0);

[RyanCavanaugh]

I could be convinced that this should be a compile-time error rather than emitting Object. Thoughts?

[andrewbranch]

It seems like it should be an error to me. But the same would apply to #37672—@rbuckton is there a reason we wouldn’t want to have an error here?

[rbuckton]

It seems at first glance like a good idea to report an error, but why should importing the type (only) of a class be any different than importing an interface and using the exact same syntax. Also, this works just fine:

import type { Services } from './services';
type S = Services.Service;

declare const decorator: any;
export class Main {
  @decorator()
  field: S;
}

So if we can use an alias or an interface, why not an import type { ... }?

I think we should treat a type-only import the same way we do an interface or type alias and just emit Object.

[liorcode]

Not sure I understand though, why should it be a compile-time error?

[andrewbranch]

I guess it depends on how important it is for the metadata to be as complete as it could be. The thought is simply that using import type here is probably a mistake, since you get better metadata just be deleting the type keyword, but the discoverability of that mistake seems super low. There is likely no reason why the user shouldn’t just be using a regular import here. But if you think the importance of the metadata is low, maybe it’s not worthwhile to error.

[liorcode]

Well, in my use case I'm importing a package using a type-only import to make sure it doesn't get bundled, since I want the bundle to be minimal. So I definitely don't want to change it to a regular import.
And actually for this file I don't really care about the metadata (I actually don't need the metadata at all in this specific file, but since I have a single tsconfig in this app, I can't control which files get it or not..).
For now, as a workaround, I just use a type alias and then it gets emitted as Object