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

Generic return value of a mixin constructor #22431

Closed
Xenya0815 opened this issue Mar 9, 2018 · 10 comments
Closed

Generic return value of a mixin constructor #22431

Xenya0815 opened this issue Mar 9, 2018 · 10 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@Xenya0815
Copy link

TypeScript Version: 2.8.0-dev.20180308

Code

interface Instance {
  foo: number;
}

interface Constructor<I extends Instance> {
  new(): I;
}

function MixIn<T extends Instance>(Base: Constructor<T>) {
  return class extends Base {
    
  }
}

Expected behavior:
That my MixIn accepts that construct so that I can define a extended interface T with properties which are available at a instance of the MixIn class.

Actual behavior:

error TS2509: Base constructor return type 'T' is not a class or interface type.

It works when I remove the generic type at the function MixIn (like function MixIn(Base: Constructor<Instance>)) .

Related Issues:
I found old issues like #8853 or #4890
Because that I thought it should work.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 9, 2018

Two things, a mixin constructor needs to have a ...args in it's signature. this way the compiler can assert that any override the new class adds is valid; see https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#support-for-mix-in-classes for mode details. Second, the return type of the constructor needs to be something that is not generic, in this case, it is a generic constraint to Instance. I think we can relax this one, but i need to verify that this is the case.

So your MixIn function should instead look like:

interface Constructor<I extends Instance> {
    new(...args: any[]): I;
}

function MixIn<T extends Constructor<Instance>>(Base: T) {
    return class extends Base {
    
    }
}   

if you want to capture the type of the instance you can use InstanceType<T> if you are using TS 2.8.

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Mar 9, 2018
@Xenya0815
Copy link
Author

thanks for your answer. In my case the Mixin is just for a special Base because the added functonality uses functions from that class. So I used the real class interface (the added functionality at my example makes no sense here, it is just to show what I mean):

function MixIn<T extends mongoose.Document, M extends mongoose.Model<T>>(Base: M) {
    return class extends Base {
      static findByName(name: string): Promise<T> {
        return this.find({name: name});
    }
}

I think that does not work because your argument " a mixin constructor needs to have a ...args in it's signature". The mongoose.Model defines the constructor as: new(doc?: any): T;
I tried to write my own Model interface with the required methods and the ...args constructor:

interface Model<T extends mongoose.Document> {
  new(...args: any[]): T;

  // copied from @types/mongoose/index.d.ts
  find(callback?: (err: any, res: T[]) => void): mongoose.DocumentQuery<T[], T>;
  find(conditions: any, callback?: (err: any, res: T[]) => void): mongoose.DocumentQuery<T[], T>;
  find(conditions: any, projection?: any | null,
    callback?: (err: any, res: T[]) => void): mongoose.DocumentQuery<T[], T>;
  find(conditions: any, projection?: any | null, options?: any | null,
    callback?: (err: any, res: T[]) => void): mongoose.DocumentQuery<T[], T>;
}

function MixIn<T extends mongoose.Document, M extends Model<T>>(Base: M) {
    return class extends Base {
      static findByName(name: string): Promise<T> {
        return this.find({name: name});
    }
}

That didn't work too. But as I changed the return value of the interface to {} instead of T, it worked... I don't understand why it don't work with T but it works... great thanks

The only disadvantage: I have to copy the interface to get the required interface with the ...args signature. Is there a better solution for it?

Also thanks for the hint with InstanceType - when I change to TS 2.8 I can remove the first generic Type.

@cspotcode
Copy link

cspotcode commented Mar 23, 2018

I think that does not work because your argument " a mixin constructor needs to have a ...args in it's signature". The mongoose.Model defines the constructor as: new(doc?: any): T; I tried to write my own Model interface with the required methods and the ...args constructor:

You don't need to write your own Model interface. It will already work with your mixin. Classes that are passed to a mixin function do not need to explicitly declare a ...args: any[] signature.

Example:

class Foo {
    constructor(a: string) {}
}
function MyMixin<C extends new (...args: any[]) => {}>(Base: C) {
    return class extends Base {
        mixinMethod() {}
    }
}
class Bar extends MyMixin(Foo) {}
}

@Xenya0815
Copy link
Author

You're right, your example works. But you've removed my requirement, that I want to call a function of the Base class. To show it, I add it to your example:

class Foo {
    constructor(a: string) {}

    doSomething() {}
}

function MyMixin<C extends new (...args: any[]) => {}>(Base: C) {
    return class extends Base {
        mixinMethod() {
            this.doSomething(); // ERROR Property 'doSomething' does not exist on type '(Anonymous class)'.
        }
    }
}

class Bar extends MyMixin(Foo) {}

Now I get an error because the doSomething is unknown - that's correct. I could add the required function to the Base class declaration:

C extends new (...args: any[]) => {doSomething(): void}

But now I'm rewriting the model interface, because a C extends Foo is not working.

@cspotcode
Copy link

@Xenya0815 You shouldn't do C extends Foo, you should do C extends Constructor<Foo>.

function MyMixin<C extends new (...args: any[]) => Foo>(Base: C) {

Does that work?

@Xenya0815
Copy link
Author

Xenya0815 commented Mar 26, 2018

ahh, you got me! you're right with instance methods. That works like a charm.
But I thought already about to do the example like my example in my previous post, with static methods.
But perhaps you have for that approach a same nice answer :-)

so once again:

class Foo {
    static doSomething() {}

    constructor(a: string) {}
}

function MyMixin<C extends new (...args: any[]) => {}>(Base: C) {
    return class extends Base {
        static mixinMethod() {
            this.doSomething(); // Property 'doSomething' does not exist on type '{ new (...args: any[]): (Anonymous class); prototype: MyMixin<any>.(Anonymous class); mixinMethod...'
        }
    }
}

class Bar extends MyMixin(Foo) {}

When I change the base class declaration to:

C extends { new(...args: any[]): {}; doSomething(): void }

It works again. Do you have a better solution for here without to define the required methods manually?

@cspotcode
Copy link

cspotcode commented Mar 26, 2018

@Xenya0815 How about doing Base.doSomething(); Does that work? EDIT: Nevermind, that doesn't make sense.

You can do Foo.doSomething();

@Xenya0815
Copy link
Author

Sure, I could use the Base directly like Foo.doSomething(). But with that I loose the possibility to use an already extended/overwritten method.

class Foo {
    static doSomething() {}

    constructor(a: string) {}
}

class ExtendedFoo extends Foo {
    static doSomething() {
        console.log('doSomething was called');
    
        super.doSomething();
    }
}

function MyMixin<C extends { new(...args: any[]): {}; doSomething(): void }>(Base: C) {
    return class extends Base {
        static mixinMethod() {
            this.doSomething(); // with log
            Foo.doSomething(); // without log
        }
    }
}

class Bar extends MyMixin(ExtendedFoo) {}

@cspotcode
Copy link

@Xenya0815 I see what you mean; that's a bummer. I devised a weird hack, does this work?

function MyMixin<C extends typeof Foo>(Base: C) {
    return (<C2 extends Constructor<Foo>>(_base: C2) => {
        return class extends _base {
            static mixinMethod() {
                Base.doSomething(); // with log
                Foo.doSomething(); // without log
            }
        }
    })(Base);
}

Base is constrained to have all the static methods, and _base is constrained to be a valid constructor. You have to call static methods as Base.doSomething(), but this lets you use the implementations of ExtendedFoo.

@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 Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants