Report use before definition for parameter types used in decorated classes and methods with --emitDecoratorMetadata

I have the following code:

@decorator
class A {
    constructor(param : B) {

    }
}

class B {

}

function decorator(target: any) {
    return target;
}

console.log(Reflect.getMetadata("design:paramtypes", A));

The expected output should be: [function B()], but the actual output is [undefined]. The issue is related to the definition order in the resulting js file, in which there is the following code:

var A = (function () {
    function A(param) {
    }
    A = __decorate([
        decorator, 
        __metadata('design:paramtypes', [B])
    ], A);
    return A;
})();
var B = (function () {
    function B() {
    }
    return B;
})();

Since A is defined by a IIFE, in its execution scope B is not yet defined, so the generated metadata is incorrectly set to undefined. The only workaround I can use now is to declare B before A.

That is correct. This is a geneneral temporal dead zone issue in javascript.

As long actual result is different from expected result, its hard to call it "this is correct".
I understand that there is not too much workforce to fixe all issues, but you should choose one of:

mark it as known issue, that maybe in future it will be fixed. You admin that current behavior differs from ideal, expected one
mark decorators feature in documentation as "sometimes might work, sometimes not" and live with that.
add compiler warning for cases when it might occurs
remove decorators feature completely.

This is not special to decorators. you can observe the same behavior in regular variables:

var A = B; // A is  undefined  and not  2
var B = 2;

similarly in classes

class A extends B { // runtime error, B is undefined
}

class B {
}

The issue is in JavaScript name bindings are hoisted to the top of the scope, but they are undefined until first initialized. This is really hard for a static analysis tool/compiler to detect correctly all the time.. as the same code can be valid if your variable was defined.

consider wrapping the first class in a function declaration as:

function setA () {
    var A = (function () {
        function A(param) {
        }
        A = __decorate([
            decorator, 
            __metadata('design:paramtypes', [B])
        ], A);
        return A;
    })();
}

// B is undefined at this point.
setA();

var B = (function () {
    function B() {
    }
    return B;
})();

// here is fine, as B is defined at the time A is evaluated
setA();

The only possible way i can think of is to make desing:paramtypes a function instead of an array, this means a dispatch call on every query, which can be expensive.

That's true when you write sequential code. statement after statement, function after function etc.
In initial example adding last line "var f = new A(new B());" results that A.ctor will have non null param argument.
In TS code there is nothing that indicates you are referring to something that is not defined yet.
Still not understand why it s correct behavior.

The problem is not the call site. it the definition site. The decorator is called only once the declaration, and when this happens, the value of desing:paramtypes is B, which is undefined at this point.

I get it that you want the compiler to be more helpful in these cases, and i was listing cases that behave similarly. We would be open to discussing suggestions here. again the only thing i can think of is making the value of desing:paramtypes either a getter, or a function that needs to be called to get the param types.

Also there has been similar design discussions here: https://github.com/jonathandturner/decorators/issues/18

Wojciech Gebczyk (@wgebczyk)

As a starting point, could we consider feasible the third option that Wojciech Gebczyk (@wgebczyk) mentioned before?

add compiler warning for cases when it might occurs

At the moment I have no idea about the complexity of this implementation, but surely could be a good way to detect at least suspicious situations like this one..

We can add a check to make sure that the definition has to lexical come before the use; similar to the current check for let/const.

compiler warning is acceptable. I understand that some features might be half baked, but I want to be notified when I'm using something that produces unexpected result.

EDIT: why not put decorators code at end of generated script? That won't solve circular dependencies, but circulars are hard without making function call instead of static population.

Wojciech Gebczyk (@wgebczyk)

Wojciech Gebczyk (@wgebczyk) My initial issue comment contained a solution similar to your proposal, but soon I edited and removed it, since there could be other unwanted side effects if some code immediately after the class declaration tries to query metadata, since it will be undefined until the "end" of the file..

So the only solution would be first generate type/function hierarchy and then fill decorators and then provide function body. Not all cycles can be solved this way.

So having compiler generate warning if spotting symbol defined below, should solve this and a few other issues.

I have a similar issue using Webpack.

File foo.ts:

import { Bar } from "./bar";

export class Foo {

    @decorator
    public bar: Bar;

}

and file bar.ts:

import { Foo } from "./foo";

export class Bar {

    @decorator
    public foo: Foo;

}

And I end up with "design:type" on Foo.bar being undefined. I did not expect this to happen using imports and Webpack, since the bundled JavaScript requires both Foo and Bar before declaring the function…

I am afraid this use case is not somethign you we can handle unless the emit-metadata switches to a getter instead of a value..

I think this would be a really common scenario, i.e. having two classes with parent/child relationship and it seems like one of the two metadata will always be undefined. I'll try to set my own "design:type" as a getter, I'll report here the outcome!

it is something we are aware of and plan to change in the next iteration of the decorators implementation.

I have managed to solve this with a new decorator which takes a getter, as suggested:

function bind(type: any): PropertyDecorator {
    return Reflect.metadata("design:type", type, target, key);
}

class Foo {

    @bind(() => Bar)
    public bar: Bar;

}

class Bar {

    @bind(() => Foo)
    public foo: Foo;

}

let typeOfFooBar = Reflect.getMetadata("design:type", Foo, "bar")(); // <= callback invocation

There doesn't seem to be an actionable suggestion here apart from whatever happens when the new decorator spec appears

Navigation Menu

Repository navigation

Report use before definition for parameter types used in decorated classes and methods with --emitDecoratorMetadata #4114

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Report use before definition for parameter types used in decorated classes and methods with --emitDecoratorMetadata #4114

Description

Activity

mhegazy commented on Aug 1, 2015

wgebczyk commented on Aug 2, 2015

mhegazy commented on Aug 2, 2015

wgebczyk commented on Aug 2, 2015

mhegazy commented on Aug 3, 2015

pcan commented on Aug 3, 2015

mhegazy commented on Aug 3, 2015

wgebczyk commented on Aug 3, 2015

pcan commented on Aug 3, 2015

wgebczyk commented on Aug 3, 2015

heruan commented on Jan 4, 2017

mhegazy commented on Jan 4, 2017

heruan commented on Jan 4, 2017

mhegazy commented on Jan 4, 2017

heruan commented on Feb 15, 2017

RyanCavanaugh commented on Jul 23, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions