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

Polymorphic "this" isn't polymorphic in some cases #5493

Closed
xealot opened this issue Nov 2, 2015 · 4 comments
Closed

Polymorphic "this" isn't polymorphic in some cases #5493

xealot opened this issue Nov 2, 2015 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@xealot
Copy link

xealot commented Nov 2, 2015

Sorry for the crap title, I'm not entirely sure what is going on but I think it's a bug. I also think this is somewhat related to #5449 but am unsure.

Library Example:

export interface InstanceConstructor<T extends BaseModel> {
    new(fac: Factory<T>): T;
}

export class Factory<T extends BaseModel> {
    constructor(private cls: InstanceConstructor<T>) {}

    get() {
        return new this.cls(this);
    }
}

export class BaseModel {
    constructor(private fac: Factory<this>) {}

    refresh() {
        // get returns a new instance, but it should be of
        // type Model, not BaseModel.
        return this.fac.get();
    }
}

User Code Example:

export class Model extends BaseModel {
    do() {
        return true;
    }
}

// Kinda sucks that Factory cannot infer the "Model" type
let f = new Factory<Model>(Model);
let a = f.get();

let b = a.refresh();

The issue that the compiler complains about is as follows:

error TS2345: Argument of type 'typeof Model' is not assignable to parameter of type 'InstanceConstructor<Model>'.
  Types of parameters 'api' and 'api' are incompatible.
    Type 'Factory<this>' is not assignable to type 'Factory<Model>'.
      Type 'this' is not assignable to type 'Model'.
        Type 'BaseModel' is not assignable to type 'Model'.
          Property 'do' is missing in type 'BaseModel'.

If I change the extended model to be specific it errors in different way, but same theme.

export class Model extends BaseModel {
    constructor(fac: Factory<Model>) {
        super(fac);
    }

    do() {
        return true;
    }
}
error TS2345: Argument of type 'Factory<Model>' is not assignable to parameter of type 'Factory<this>'.
  Type 'Model' is not assignable to type 'this'.

I think the reason is because this isn't being polymorphic in either the template or the constructor. When this is analyzed in the constructor of BaseModel it seems to stick with that context.

I initially posted about the use of polymorphic this which was answered by @RyanCavanaugh http://stackoverflow.com/questions/33443793/create-a-generic-factory-in-typescript

@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

this looks like a duplicate of #5449, this types are not supported in constructor arguments, nor is it supported in static methods parameters or return types. #5449 tracks adding the correct error message.

@mhegazy mhegazy closed this as completed Nov 2, 2015
@mhegazy mhegazy added the Duplicate An existing issue was already created label Nov 2, 2015
@xealot
Copy link
Author

xealot commented Nov 2, 2015

So this use case is not supported? Based on my example is there any other way to accomplish what I'm attempting? This isn't an unusual pattern for Resource/Model implementation.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

So this use case is not supported?

that is correct. constructors are really part of the static side of classes, and for these this types do not apply.

Based on my example is there any other way to accomplish what I'm attempting? This isn't an unusual pattern for Resource/Model implementation.

As mentioned earlier, this was not part of the original implementation. we should reconsider thought.

@xealot
Copy link
Author

xealot commented Nov 2, 2015

Fortunately the type system still seems to inspect and output things correctly so this isn't slowing me down much which is great. I'll keep an eye on these two issues for updates.

That was a lie. It seems confused at best :(

@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

2 participants