Skip to content
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

Allow type annotations on constructors #13355

Closed
MrMatthewLayton opened this issue Jan 8, 2017 · 6 comments
Closed

Allow type annotations on constructors #13355

MrMatthewLayton opened this issue Jan 8, 2017 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@MrMatthewLayton
Copy link

I have a particular case (very possibly edge case) where I want to define a class that takes a template, and merges the template into the classes object instance, thus, the instance would be an intersection of my template and the class itself.

My current code works but has some limitations:

interface Dynamic {
    foo: any;
}

interface DynamicConstructor<T extends {}> {
    new <T>(template?: T): T & Dynamic;
}

declare var Dynamic: DynamicConstructor<any>

var msg = new Dynamic({
    message: "Hello, world",
    sentDate: new Date(2017, 5, 4)
});

msg.message = "Something else";
msg.sentDate.setFullYear(2018);
msg.foo = "bar";

interface IPerson {
    firstName: string;
    lastName: string;
}

var person: IPerson = new Dynamic<IPerson>({
    firstName: "John",
    lastName: "Doe"
});

person.firstName = "Jack";
person.lastName = "Smith";

person.foo = "bar"
// ^ Not allowed as "person" is scoped to IPerson

View in the Playground

The limitations here are that because I can't define a constructor that returns an intersection type, I cannot define a class at all, therefore I lose the ability to use TypeScript's inheritance. This is because I have to implement the functionality exposed by these interfaces manually, using JavaScript.

Therefore I would like TypeScript to allow type annotations on constructors (perhaps the limitation here should be that the annotation should be an intersection type which includes the class)

class Dynamic<T extends{}> {
    constructor(template?: T) Dynamic & T {
        ...
    }

    ... The rest of the implementation ...
}

In conclusion, I could therefore Implement the class correctly, allowing the class to use inheritance.

As another point related to this, the current declaration model in TypeScript needs revising, because it makes it appear that you can construct interfaces that have no implementation. Consider this from lib.d.ts

interface Object {
    ...
}

interface ObjectConstructor {
    new(): Object;
}

declare var Object: ObjectConstructor;

Object at this point appears like an interface, but can be instantiated and not inherited. This is not natural in OOP. It would be better if the declaration could be...

declare class Object {
    constructor(): Object;
    constructor(...): Object;
    ^ ability to add multiple constructor declarations, but only for ambient declarations...perhaps.
}
@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jan 9, 2017

I believe this is a duplicate of #11588 #4890 (thanks @mhegazy). With respect to your example, the problem is the type annotation on the declaration of person there are no errors if you remove it.

@MrMatthewLayton
Copy link
Author

MrMatthewLayton commented Jan 9, 2017

@aluanhaddad the problem is not with the type annotation on the person declaration...I see where you're going with that, but that's not the issue. The problem is that I'm constructing a new object of type Dynamic<T> where T is a template. The template contains key/value pairs that are "dynamically" bound into the new instance, thus, the resulting type is an intersection of Dynamic<T> & T

Say for example, Dynamic<T> is implemented like this:

class Dynamic<T extends {}> {
    public readonly foo: any;
    public readonly bar: any;
    constructor(template?: T) {
    }
}

var x = new Dynamic();

x.foo and x.bar are in scope as members of the Dynamic<T> instance, but now I want to intersect a template, which is a dictionary of key/value pairs into the Dynamic<T> instance. This is done with Object.defineProperty

class Dynamic<T extends {}> {
    public readonly foo: any;
    public readonly bar: any;
    constructor(template?: T) {
        if (!!template) {
            Object.keys(template).forEach(key => {
                Object.defineProperty(this, key, {
                    get: function () { return this[key]; },
                    set: function (val: any) { this[key] = val; },
                    enumerable: true,
                    configurable: false
                });

                this[key] = template[key];
            });
        }
    }
}

var x = new Dynamic({
    something: 1,
    somethingElse: 2,
    anotherOne: "Blah"
});

x.foo... // works!
x.bar... // works!
x.something... // doesn't work in TypeScript, but works in JavaScript.

Therefore, I need to be able to annotate the constructor to make it clear to typescript that the constructor generates Dynamic<T> & T.

Essentially this boils down to the fact that I can annotate new in interfaces but not constructor in classes..

This works...

interface DynamicConstructor<T extends {}> {
    new <T>(template?: T): T & Dynamic<T>;
}

interface Dynamic<T> {
}

This doesn't...

declare class Dynamic<T> {
    constructor(template?: T): Dynamic<T> & T;
}

Possible solutions I can see...

  1. Relax the type annotation restriction on constructors to allow annotations to be restricted to : ThisType or :ThisType & OtherType & ...

  2. Allow this to be handled with declaration merging, where constructor parameters match:

interface Dynamic<T extends {}> {
    new <T>(template?: T): Dynamic<T> & T;
    // ^ Defines the constructor for Dynamic<T> with type annotation for return type.
}

class Dynamic<T extends {}> {
    constructor(template?: T) { /*...*/ }
    // ^ Implements the constructor for Dynamic<T>, but compiler infers return type from the interface.
}

@mhegazy
Copy link
Contributor

mhegazy commented Jan 9, 2017

Duplicate of #4890?

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@vvakame
Copy link
Contributor

vvakame commented Jul 8, 2017

I want to add type annotation to constructor too.
I wrote above code. but TypeScript can't handle this pattern I think.

function loooooongTask() {
    return new Promise<string>(resolve => {
        setTimeout(() => {
            resolve("world");
        }, 3000);
    })
}

class Foo {
    name: string;
    constructor() { // I want add type assertion to here. Promise<Foo>. 
        return loooooongTask().then(v => {
            this.name = v;
            return this;
        })
    }

    hello() {
        console.log(`Hello, ${this.name}`);
    }
}

(async () => {
    const obj = await new Foo();
    obj.hello();
})();

@aluanhaddad
Copy link
Contributor

@vvakame see #11588 (comment) for the rationale behind not allowing this.

@mhegazy mhegazy added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 20, 2017
@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants