Skip to content

loading, declaration and execution phases of modules and cyclic references problem ? #7282

Closed

Description

@casser

This question is related to TypeScript, ES6, SystemJs and ReflectJs specs, but I would like to know your thoughts about it.

So for a function declarations the sequence of code is not important, but for classes it is:

Example with single module

console.info(fnc()); // this is fine
console.info(new Cls()); //TypeError: Cls is not a constructor
function fnc(){
  return "value"
}
class Cls {
  constructor(){
    //...
  }
}

A more complex example when classes located in different modules and have cross references in declaration phase like:

Example with multiple modules and static references

// one.ts
import {Two} from "./two"
class One {
  static two = new One();
}
// two.ts
import {One} from "./one"
class One {
  static two = new One(); //TypeError: One is not a function 
}
//main.ts
import {One} from "./one"
import {Two} from "./two"
console.info(One.two) 
console.info(Two.one)

so this is throwing error when trying to create One instace, the expectation is to have One declared at the time of exection .

This can be hacked by using some cached getters, which can be somehow used to solve the main problem in runtime(loader) :

// cached.ts
export function Cached(target,key,desc){
    var initializer = desc.get;
    desc.get = function(){
        return Object.defineProperty(this,key,{
            value:initializer()
        })[key];
    };
    return desc;
}
// one.ts
import {Cached} from "./cached";
import {Two} from "./two";
export class One {
    @Cached
    static get two():Two{
        return new Two()
    }
}
// two.ts
import {Cached} from "./cached";
import {One} from "./one";
export class Two {
    @Cached
    static get one():One{
        return new One();
    }
}
//main.ts
import {One} from "./one"
import {Two} from "./two"
console.info(One.two) 
console.info(Two.one)

I have prepared some demo to show hack on this problem, you can see bellow

Expected behavior:

Create and instance of Cls and out put it without errors.

Actual behavior:

Throws a TypeError because of undeclared class

Example of cyclic references problem in decoration phase

The other example is annotators cyclic references problem.

function Model(target){}
function Collection(target){}
function Field(target:any,key:any,desc?:any):any{
    var type:any = Reflect.getMetadata("design:type",target,key);
    document.write(`<pre>${target.constructor.name}.${key} : ${type?type.name:"<span style='color: red'>UNDEFINED ?</span>"}</pre>`);
}

@Model
export class User {
    @Field id:String;
    @Field name:String;
    @Field projects:Projects; // Project Is not defined yet
}

@Collection
export class Users extends Array<User> {}

@Model
export class Project {
    @Field id:String;
    @Field name:String;
    @Field author:User;
    @Field contributors:Users; // Users will not be defined 
                               // if we will change definition order
}

@Collection
export class Projects extends Array<Project>{}

I think this is IMPORTANT because Dependecy Injection (java guice), ORM's or Serialization frameworks will use this feature deeply, and user code can contain cross referenced types.

So the main reason for this problem is that declaration, decoration and execution phases of classes mixed in output.

// decoration inside execution block and part of class declaration 
__decorate([
    Field, 
    __metadata('design:type', Projects)
], User.prototype, "projects", void 0);

Potencial hack for this can be generate closure instead of direct type referance in metadata like:

__decorate([
    Field, 
    __metadata('design:type', function(){return Projects})
], User.prototype, "projects", void 0);

which can be adjusted after module execution, but I think this is just a hack, and the right way will be finnaly define
specification about Loading > Declaration > Annotation > Decoration and Execution phases of runtime which will support cyclic references.

Any thoughts ?

PS: AND AGAIN THANKS FOR TYPESCRIPT :)

Activity

  1. DanielRosenwasser commented on Feb 28, 2016

    @DanielRosenwasser
    Member

    Sergey Mamyan (@casser) so I'm having a hard time reading through this post and finding the question 😄 - can you clarify what exactly you're asking here?

  2. casser commented on Feb 29, 2016

    @casser
    Author

    I just realize that links was broken, sorry :)

    Question: Examples above are bugs or expected behaviors ?

    new Class(); //TypeError: Cls is not a constructor
    class Class {
      constructor(){
        console.info("Created")
      }
    }

    Expected behavior:

    output: Created
    

    Actual behavior: Demo

    output: TypeError: Cls is not a constructor
    
    function Field(target,key,desc?){
      console.info(Reflect.getMetadata("design:type",target,key).name)
    }
    class One {
      @Field field:Two;
    }
    class Two {
      @Field field:One;
    }

    Expected behavior:

    output:
    Two
    One
    

    Actual behavior: Demo

    output:
    undefined
    One
    

    Look At this repo for sources

  3. DanielRosenwasser commented on Feb 29, 2016

    @DanielRosenwasser
    Member

    Ah, gotcha. Ron Buckton (@rbuckton) would know a lot better about the decorators. Though, is #4114 related?

    As for using a class before its definition, yes, we should be reporting an error. See #21, #4341, and #5207. Apparently there was a fix out for it, but it was never taken in (#4343). Mohamed Hegazy (@mhegazy) what's going on with that?

  4. mhegazy commented on Feb 29, 2016

    @mhegazy
    Contributor

    As for using a class before its definition, yes, we should be reporting an error. See #21, #4341, and #5207. Apparently there was a fix out for it, but it was never taken in (#4343). Mohamed Hegazy (@mhegazy) what's going on with that?

    #5207 is the correct issue tracking this change

  5. rbuckton commented on Feb 29, 2016

    @rbuckton
    Contributor

    This is a known issue with the --emitDecoratorMetadata switch, and your proposed solution is exactly what we have discussed as the resolution. Regardless of "use before definition" type errors, using --emitDecoratorMetadata would still emit JavaScript that will result in an error in ES6 unless we wrap the metadata values in a function.

  6. pie6k commented on Oct 30, 2017

    @pie6k

    Any solution to this? How can I wrap values of metadata in a functions?

    It's very blocking for any project with decorators that have circular references.

  7. locked and limited conversation to collaborators on Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DuplicateAn existing issue was already created

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions