Skip to content

Emit fallback for decorator metadata for type only imports #39337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36973,29 +36973,31 @@ namespace ts {
}

// Resolve the symbol as a value to ensure the type can be reached at runtime during emit.
const valueSymbol = resolveEntityName(typeName, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, location);
const valueSymbol = resolveEntityName(typeName, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ true, location);
const isTypeOnly = valueSymbol?.declarations?.every(isTypeOnlyImportOrExportDeclaration) || false;
const resolvedSymbol = valueSymbol && valueSymbol.flags & SymbolFlags.Alias ? resolveAlias(valueSymbol) : valueSymbol;

// Resolve the symbol as a type so that we can provide a more useful hint for the type serializer.
const typeSymbol = resolveEntityName(typeName, SymbolFlags.Type, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, location);
if (valueSymbol && valueSymbol === typeSymbol) {
if (resolvedSymbol && resolvedSymbol === typeSymbol) {
const globalPromiseSymbol = getGlobalPromiseConstructorSymbol(/*reportErrors*/ false);
if (globalPromiseSymbol && valueSymbol === globalPromiseSymbol) {
if (globalPromiseSymbol && resolvedSymbol === globalPromiseSymbol) {
return TypeReferenceSerializationKind.Promise;
}

const constructorType = getTypeOfSymbol(valueSymbol);
const constructorType = getTypeOfSymbol(resolvedSymbol);
if (constructorType && isConstructorType(constructorType)) {
return TypeReferenceSerializationKind.TypeWithConstructSignatureAndValue;
return isTypeOnly ? TypeReferenceSerializationKind.TypeWithCallSignature : TypeReferenceSerializationKind.TypeWithConstructSignatureAndValue;
}
}

// We might not be able to resolve type symbol so use unknown type in that case (eg error case)
if (!typeSymbol) {
return TypeReferenceSerializationKind.Unknown;
return isTypeOnly ? TypeReferenceSerializationKind.ObjectType : TypeReferenceSerializationKind.Unknown;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know the type is objecty? You can import type a primitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeSerializationKind.ObjectType tells the ts transform to emit Object for the type metadata (which essentially means any). TypeSerializationKind.Unknown means we should try to resolve a value at runtime using that name (by using typeof testing), which would be incorrect behavior. Consider the following using the current version of the compiler (without this change):

// ts
import { dec } from "./decorators";
import type { Map } from "./myMap";

@dec
export class C {
  constructor(map: Map) {}
}

// js
...
let C = /** @class */ (() => {
    var _a;
    let C = class C {
        constructor(map) { }
    };
    C = __decorate([
        dec,
        __metadata("design:paramtypes", [typeof (_a = typeof Map !== "undefined" && Map) === "function" ? _a : Object])
    ], C);
    return C;
})();

The design:paramtypes metadata for C will incorrectly use the global Map object.

For a type-only import we can be certain that the type cannot be referenced as a value, so we should emit Object instead of trying to resolve a value for it.

}
const type = getDeclaredTypeOfSymbol(typeSymbol);
if (type === errorType) {
return TypeReferenceSerializationKind.Unknown;
return isTypeOnly ? TypeReferenceSerializationKind.ObjectType : TypeReferenceSerializationKind.Unknown;
}
else if (type.flags & TypeFlags.AnyOrUnknown) {
return TypeReferenceSerializationKind.ObjectType;
Expand Down
60 changes: 60 additions & 0 deletions tests/baselines/reference/decoratorMetadataWithTypeOnlyImport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//// [tests/cases/conformance/decorators/decoratorMetadataWithTypeOnlyImport.ts] ////

//// [service.ts]
export class Service {
}
//// [component.ts]
import type { Service } from "./service";

declare var decorator: any;

@decorator
class MyComponent {
constructor(public Service: Service) {
}

@decorator
method(x: this) {
}
}

//// [service.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Service = void 0;
var Service = /** @class */ (function () {
function Service() {
}
return Service;
}());
exports.Service = Service;
//// [component.js]
"use strict";
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = (this && this.__metadata) || function (k, v) {
if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
Object.defineProperty(exports, "__esModule", { value: true });
var MyComponent = /** @class */ (function () {
function MyComponent(Service) {
this.Service = Service;
}
MyComponent.prototype.method = function (x) {
};
__decorate([
decorator,
__metadata("design:type", Function),
__metadata("design:paramtypes", [Object]),
__metadata("design:returntype", void 0)
], MyComponent.prototype, "method", null);
MyComponent = __decorate([
decorator,
__metadata("design:paramtypes", [Function])
], MyComponent);
return MyComponent;
}());
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
=== tests/cases/conformance/decorators/service.ts ===
export class Service {
>Service : Symbol(Service, Decl(service.ts, 0, 0))
}
=== tests/cases/conformance/decorators/component.ts ===
import type { Service } from "./service";
>Service : Symbol(Service, Decl(component.ts, 0, 13))

declare var decorator: any;
>decorator : Symbol(decorator, Decl(component.ts, 2, 11))

@decorator
>decorator : Symbol(decorator, Decl(component.ts, 2, 11))

class MyComponent {
>MyComponent : Symbol(MyComponent, Decl(component.ts, 2, 27))

constructor(public Service: Service) {
>Service : Symbol(MyComponent.Service, Decl(component.ts, 6, 16))
>Service : Symbol(Service, Decl(component.ts, 0, 13))
}

@decorator
>decorator : Symbol(decorator, Decl(component.ts, 2, 11))

method(x: this) {
>method : Symbol(MyComponent.method, Decl(component.ts, 7, 5))
>x : Symbol(x, Decl(component.ts, 10, 11))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
=== tests/cases/conformance/decorators/service.ts ===
export class Service {
>Service : Service
}
=== tests/cases/conformance/decorators/component.ts ===
import type { Service } from "./service";
>Service : Service

declare var decorator: any;
>decorator : any

@decorator
>decorator : any

class MyComponent {
>MyComponent : MyComponent

constructor(public Service: Service) {
>Service : Service
}

@decorator
>decorator : any

method(x: this) {
>method : (x: this) => void
>x : this
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// @experimentalDecorators: true
// @emitDecoratorMetadata: true
// @target: es5
// @module: commonjs
// @filename: service.ts
export class Service {
}
// @filename: component.ts
import type { Service } from "./service";

declare var decorator: any;

@decorator
class MyComponent {
constructor(public Service: Service) {
}

@decorator
method(x: this) {
}
}