Classes passed to decorators dont work when there circular references used in those classes

[pleerock]

First I thought the problem in reflect-metadata component I used, but now I think the issue is with typescript decorators themself. I described that issue there: microsoft/reflect-metadata#12

You can understand what is problem from my code out run output:

import {Person} from "./Person";
import {Relation} from "./Relation";

export class PersonInfo {
    @Relation(Person)
    info: string;
}

import {Relation} from "./Relation";
import {PersonInfo} from "./PersonInfo";

export class Person {
    name: string;
    @Relation(PersonInfo)
    info: string;
}

export function Relation(cls) {
    return function () {
        console.log(cls);
    }
}

import {Person} from "./Person";
let person = new Person();

result:

undefined
[Function: PersonInfo]

expected result:

[Function: Person]
[Function: PersonInfo]

From generated sources I found that Person_1.Person is undefined here:

    __decorate([
        Relation_1.Relation(Person_1.Person), 
        __metadata('design:type', String)
    ], PersonInfo.prototype, "info");

[weswigham]

@rbuckton

[rbuckton]

We've discussed this issue amongst the team in the past. We may need to change our __metadata helper to supply a function that when executed would return the metadata instead. This would help somewhat with issues with load order.

If we decide to go with that solution, the output from your example in microsoft/reflect-metadata#12 would instead look as follows:

__decorate([
    Relation_1.Relation(),
    __metadata('design:type', function() { return Person_1.Person; })
], PersonInfo.prototype, "info");

Then your call to Reflect.getMetadata would change to something like:

  let typefn = Reflect.getMetadata('design:type', object, propertyName);
  let type = typefn();

You still wouldn't be able to get the 'design:type' if the constructor from the external file has still not yet been exported, but there is no way around that given the execution semantics of JavaScript.

[weswigham]

In this case, the issue is with

Relation_1.Relation(Person_1.Person) //Person_1.Person is undefined

not

__metadata('design:type', String) //This is perfectly fine

Metadata is immaterial to the issue - the decorator is referencing what should be a resolved import as an argument, but at decoration time it is not yet resolved, for some reason. What's going on is that the .execute part of the system module is getting executed before all of its dependency slots have finished loading because the dependency is circular. Since decorators are eager, there's no time for it to finish defining itself and let the other module load so it's defined for the decorator call.

[rbuckton]

@weswigham This is again due to execution order. If you look at a simpler example:

var a = function(b) { console.log(typeof b); }
a(b); // "undefined";
var b = function(a) { console.log(typeof a); }
b(a); // "function";

Decorators do not change the fact that while the BindingIdentifier for a VariableDeclaration or ClassDeclaration are hoisted, the initializers or class bodies are not evaluated until execution proceeds to the point where they are declared. Imports and exports don't really change this underlying behavior, so circular references behave in the same fashion. In fact, the only declaration that hoists the body of the declaration is a FunctionDeclaration. As such, the same example above written using function declarations has different output:

function a(b) { console.log(typeof b); }
a(b); // "function";
function b(a) { console.log(typeof a); }
b(a); // "function";

Since _ClassDeclaration_s and _VariableDeclaration_s don't hoist their bodies or initializers, the only way to handle this out of order is to enclose the references in a function closure that can be evaluated later, after all class bodies or initializers (or exports in a circular module dependency graph) have been evaluated. As such, @pleerock's Relation decorator would need to take a function instead:

// Relation.ts
let relationMap = new WeakMap<any, Map<string, () => Function>>();
export function Relation(typefn: () => Function) {
  return function (target, propertyName) {
    // Calling `typefn` here may not give you the value, you need to 
    // store it somewhere and evaluate it later...
    let map = relationMap.get(target);
    if (!map) relationMap.set(target, map = new Map<string, () => Function>());
    map.set(propertyName, typefn);
  }
}

export function getRelation(target, propertyName) {
  while (target) {
    let map = relationMap.get(target);
    if (map) {
      let typefn = map.get(propertyName);
      if (typefn) {
        return typefn();
      }
    }
    target = Object.getPrototypeOf(target);
  }
}

// PersonInfo.ts
import { Person } from './Person';
import { Relation } from './Relation';
export class PersonInfo {
  @Relation(() => Person)
  info: string;
}

// Person.ts
import { PersonInfo } from './PersonInfo';
import { Relation } from './Relation';
export class Person {
  @Relation(() => PersonInfo)
  info: string;
}

// App.ts
import { Person } from './Person';
import { PersonInfo } from './PersonInfo';
import { getRelation } from './Relation';

var a = new Person();
var b = new PersonInfo();
getRelation(a, "info"); // PersonInfo
getRelation(b, "info"); // Person

[Verdier]

@rbuckton is there any news on that? Looks like the issue is closed but I've not found any other issue to track this. Does the By Design label from @mhegazy means that the problem will never be solved as you suggest in your comments?

[meirgottlieb]

@rbuckton, I understand this issue is closed, but I just wanted to point out that this same problem exists with the type information provided by the emitDecoratorMetadata compiler flag. This is a slightly different problem because it can't be fixed by the user passing a factory function to the decorator since the code is generated by the compiler.

[gusev-p]

I experienced this same issue. @rbuckton, why compiler couldn't delay call to __decorate/__metadata untill all classes are defined? Something like

var A = (function () {
    function A() {
    }

    return A;
}());
var B = (function () {
    function B() {
    }
    return B;
}());
__decorate([
        json, 
        __metadata('design:type', B)
    ], A.prototype, "b", void 0);

instead of

var A = (function () {
    function A() {
    }
    __decorate([
        json, 
        __metadata('design:type', B)
    ], A.prototype, "b", void 0);
    return A;
}());
var B = (function () {
    function B() {
    }
    return B;
}());

[pleerock]

currently I think its one of the biggest design issue we have. We all are forced to use monkey patching like functions that returns types, or for example angular team uses fowardRef function