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

Can't extend intersection containing constructable type parameter #22824

Open
canonic-epicure opened this issue Mar 23, 2018 · 10 comments
Open
Labels
Bug A bug in TypeScript
Milestone

Comments

@canonic-epicure
Copy link

canonic-epicure commented Mar 23, 2018

Lets say we have 2 mixins, like this:

type Constructable<T = {}> = new (...args : any[]) => T

export const Mixin1 = <T extends Constructable>(base : T) =>

class Mixin1 extends base {
    method1() {
    }
}

export const Mixin2 = <T extends Constructable>(base : T) =>

class Mixin2 extends base {
    method2 () {
    }
}

Now we want to declare a Mixin3 that already includes Mixin2:

  • First of all this won't work
export const Mixin3 = <T extends Constructable>(base : T) =>

class Mixin3 extends Mixin2(base) { // !!! typecheck error here

    method3 () {
    }
}

but this form will work

export const Mixin3 = <T extends Constructable>(base : T) =>

class Mixin3 extends Mixin2<Constructable>(base) {
    method3 () {
    }
}

This is generally fine.

Now, we want to declare Mixin4 which includes Mixin3 and Mixin1:

export const Mixin4 = <T extends Constructable>(base : T) =>

class Mixin4 extends Mixin3(Mixin1(base)) {

    method4 () {
        this.method1() // !!! this method is not "recognized"

        this.method2() // !!! this method is recognized
    }
}

Note, that for Mixin4 the specification of the form Mixin2<Constructable> like we did for Mixin3 is not needed.

It seems the nested application of the Mixin1 in the declaration of Mixin4 was not "picked up". Any help is greatly appreciated.

=======================================================================
All the code from above in one snippet:

type Constructable<T = {}> = new (...args : any[]) => T

export const Mixin1 = <T extends Constructable>(base : T) =>

class Mixin1 extends base {
    method1() {
    }
}

export const Mixin2 = <T extends Constructable>(base : T) =>

class Mixin2 extends base {
    method2 () {
    }
}


export const Mixin3 = <T extends Constructable>(base : T) =>

class Mixin3 extends Mixin2<Constructable>(base) {
    method3 () {
    }
}


export const Mixin4 = <T extends Constructable>(base : T) =>

class Mixin4 extends Mixin3(Mixin1(base)) {
    method4 () {
        this.method1()

        this.method2()
    }
}
@canonic-epicure
Copy link
Author

This form does not work either:

export const Mixin4 = <T extends Constructable>(base : T) => {

    class Temp extends Mixin1<Constructable>(base) {
        methodTemp () {
            this.method1() // typechecks fine
        }
    }

    return class Mixin4 extends Mixin3(Temp) {

        method4 () {
            this.method1() // !!! not found

            this.method2()
        }
    }
}

@canonic-epicure
Copy link
Author

canonic-epicure commented Mar 23, 2018

Huh, but changing the order of mixin applications fixes it:

export const Mixin4 = <T extends Constructable>(base : T) =>

//  class Mixin4 extends Mixin3(Mixin1(base)) { -- fails in this order
    class Mixin4 extends Mixin1(Mixin3(base)) { // -- works in this order
        method4 () {
            this.method1() // method found

            this.method2() // method found
        }
    }

Ehm, why does the order of mixins application matter in this case? Definitely feels like some quirk in the typechecker.

@canonic-epicure canonic-epicure changed the title [2.8.0-rc] [Question] Nested mixin application does not work [2.8.0-rc] [Question] Nested mixin application does not work / The order of mixins application produces different typecheck results Mar 23, 2018
@ghost
Copy link

ghost commented Mar 23, 2018

The bug is that we don't consider an intersection containing a constructable type parameter to be constructable:

type Ctr = new (...args : any[]) => any;
const Mixin1 = <T extends Ctr>(Sup: T) => class extends Sup {};
const Mixin2 = <T extends Ctr>(Sup: T) => class extends Mixin1(Sup) {};

@ghost ghost changed the title [2.8.0-rc] [Question] Nested mixin application does not work / The order of mixins application produces different typecheck results Can't extend intersection containing constructable type parameter Mar 23, 2018
@ghost ghost added the Bug A bug in TypeScript label Mar 23, 2018
@canonic-epicure
Copy link
Author

Cool, hopefully the fix will make it to 2.8.0 final..

@canonic-epicure
Copy link
Author

Any workarounds possible to use right now?

@ghost
Copy link

ghost commented Mar 23, 2018

It doesn't look like there's a good workaround, besides avoiding mixins extending mixins and having the final ("leaf") class declarations directly write all the mixins they want to include.

@canonic-epicure
Copy link
Author

I see. In our project we primarily use mixins for functionality extension (a-la haskell typeclasses). There's not many "leaf" class declarations actually - those are in the external project, that consumes required parts of the functionality from ours.

@canonic-epicure
Copy link
Author

canonic-epicure commented Mar 23, 2018

Some thoughts:

The test case you posted:

type Ctr = new (...args : any[]) => any;
const Mixin1 = <T extends Ctr>(Sup1: T) => class extends Sup1 {};
const Mixin2 = <T extends Ctr>(Sup2: T) => class extends Mixin1(Sup2) {};

Can be fixed with explicit type for the application of generic function Mixin1<T>:

type Ctr = new (...args : any[]) => any;
const Mixin1 = <T1 extends Ctr>(Sup1: T1) => class extends Sup1 {};
const Mixin2 = <T2 extends Ctr>(Sup2: T2) => class extends Mixin1<Ctr>(Sup2) {};

Shouldn't that be a job of type inference facilities? Since Sup2 : T2 and T2 extends Ctr type checker has enough information to infer, that T in Mixin1<T> is of the type T2 extends Ctr, which should give required type annotation.

So the bug is probably in the code for type inference for generic functions.

@ghost
Copy link

ghost commented Mar 23, 2018

Ctr isn't the right type argument though, T2 is -- you don't want to extend just any constructor, you want the particular type that was passed in. If you use Mixin1<Ctr> you can't call a method defined by Sup2, because the type information is lost.

@canonic-epicure
Copy link
Author

canonic-epicure commented Mar 23, 2018

Right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
Rolling Work Tracking
  
Not started
Development

No branches or pull requests

6 participants