microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.2k stars 12.38k forks source link

feature request: support for mixins composed from other mixins. #32080

Open trusktr opened 5 years ago

trusktr commented 5 years ago

Search Terms

Suggestion

At the moment, it seems to be very difficult to compose mixins from other mixins.

Here's an example on StackOverflow: https://stackoverflow.com/questions/56680049

Here's an example on playground.

The code:

type Constructor<T = any, A extends any[] = any[]> = new (...a: A) => T

function FooMixin<T extends Constructor>(Base: T) { 
    return class Foo extends Base { 
        foo = 'foo'
    }
}

function BarMixin<T extends Constructor>(Base: T) { 
    return class Bar extends FooMixin(Base) { 
        test() { 
            console.log(this.foo) //  PROBLEM: this.foo is 'any' =(
        }
    }
}

Use Cases

To make it simpler to make mixins (and compose them) like we can in plain JavaScript.

I'm porting JavaScript code to TypeScript, and the JavaScript makes great use of mixins (including composing new mixins from other mixins), but the composition ispractically impossible to do in TypeScript without very tedious type casting.

Examples

Here is the plain JS version of the above example:

function FooMixin(Base) { 
    return class Foo extends Base { 
        foo = 'foo'
    }
}

function BarMixin(Base) { 
    // BarMixin is composed with FooMixin
    return class Bar extends FooMixin(Base) { 
        test() { 
            console.log(this.foo) // this.foo is obviously inherited from FooMixin!
                           // ^--- This shoud not be an error!
        }
    }
}

It seems to me, that the type checker can realize that the class returned from FooMixin(Base) will be a typeof Foo. The type system could at least be able to allow the Bar class to use methods and properties from Foo, despite not knowing what the Base class will be.

You can also imagine this problem gets worse with more composition, f.e.

    return class Bar extends Foo(Baz(Lorem(Ipsum(Base)))) {

It should also be possible to constrain the constructor to inherit from a certain base class. For example, the following doesn't work:

(EDIT: this part may actually be moved to a separate issue) (EDIT 2: this part seems to be resolved)

// Think about Custom Elements here:
function FooMixin<T extends typeof HTMLElement>(Base: T) { 
    return class Foo extends Base { 
        test() {
            this.setAttribute('foo', 'bar')
        }
    }
}

[playground link](http://www.typescriptlang.org/play/#src=function%20FooMixin%3CT%20extends%20typeof%20HTMLElement%3E(Base%3A%20T)%20%7B%20%0D%0A%0D%0A%20%20%20%20return%20class%20Foo%20extends%20Base%20%7B%20%0D%0A%20%20%20%20%20%20%20%20test()%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20this.setAttribute('foo'%2C%20'bar')%0D%0A%20%20%20%20%20%20%20%20%7D%0D%0A%20%20%20%20%7D%0D%0A%0D%0A%7D)

As @dragomirtitian pointed out on SO, there are workarounds, but they appear to be very complicated and impractical.

Here's a more realistic example of what I'm doing in JS (and trying to port to TS): I'm using a Mixin() helper function, as a type declaration for the following example, which in practice implements things like Symbol.hasInstance to check if instances are instanceof a given mixin, prevents duplicate mixin applications, and other features, but the types don't work:

type Constructor<T = any, A extends any[] = any[]> = new (...a: A) => T

type MixinFunction = <TSuper>(baseClass: Constructor<TSuper>) => Constructor<TSuper>

// this function does awesome: ensures mixins aren't applied
// more than once on a prototype chain, sets up Symbol.hasInstance so that
// instanceof checks works with any mixin application, etc.
declare function Mixin<T extends MixinFunction>(
    mixinFn: T,
    DefaultBase?: Constructor
): ReturnType<T> & {mixin: T}

function FooMixin<T extends Constructor>(Base: T) { 
    return class Foo extends Base { 
        foo = 'foo'
    }
}

const Foo = Mixin(FooMixin)
type Foo = typeof Foo

function BarMixin<T extends Constructor>(Base: T) { 
    return class Bar extends Foo.mixin(Base) {
        bar = 'bar'

        test() {
            this.foo = 'foofoo' // should work!
        }
    }
}

const Bar = Mixin(BarMixin)

class Baz extends Bar {

    test() {
        this.bar = 'barbar' // should work!
        this.foo = 'foofoo' // should work!
    }

}

const f: Foo = new Bar()

[playground link](http://www.typescriptlang.org/play/#src=type%20Constructor%3CT%20%3D%20any%2C%20A%20extends%20any%5B%5D%20%3D%20any%5B%5D%3E%20%3D%20new%20(...a%3A%20A)%20%3D%3E%20T%0D%0A%0D%0Atype%20MixinFunction%20%3D%20%3CTSuper%3E(baseClass%3A%20Constructor%3CTSuper%3E)%20%3D%3E%20Constructor%3CTSuper%3E%0D%0A%0D%0A%2F%2F%20this%20function%20does%20awesome%3A%20ensures%20mixins%20aren't%20applied%0D%0A%2F%2F%20more%20than%20once%20on%20a%20prototype%20chain%2C%20sets%20up%20Symbol.hasInstance%20so%20that%0D%0A%2F%2F%20instanceof%20checks%20works%20with%20any%20mixin%20application%2C%20etc.%0D%0Adeclare%20function%20Mixin%3CT%20extends%20MixinFunction%3E(%0D%0A%20%20%20%20mixinFn%3A%20T%2C%0D%0A%20%20%20%20DefaultBase%3F%3A%20Constructor%0D%0A)%3A%20ReturnType%3CT%3E%20%26%20%7Bmixin%3A%20T%7D%0D%0A%0D%0Afunction%20FooMixin%3CT%20extends%20Constructor%3E(Base%3A%20T)%20%7B%20%0D%0A%20%20%20%20return%20class%20Foo%20extends%20Base%20%7B%20%0D%0A%20%20%20%20%20%20%20%20foo%20%3D%20'foo'%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Aconst%20Foo%20%3D%20Mixin(FooMixin)%0D%0Atype%20Foo%20%3D%20typeof%20Foo%0D%0A%0D%0A%0D%0Afunction%20BarMixin%3CT%20extends%20Constructor%3E(Base%3A%20T)%20%7B%20%0D%0A%20%20%20%20return%20class%20Bar%20extends%20Foo.mixin(Base)%20%7B%0D%0A%20%20%20%20%20%20%20%20bar%20%3D%20'bar'%0D%0A%0D%0A%20%20%20%20%20%20%20%20test()%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20this.foo%20%3D%20'foofoo'%20%2F%2F%20should%20work!%0D%0A%20%20%20%20%20%20%20%20%7D%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Aconst%20Bar%20%3D%20Mixin(BarMixin)%0D%0A%0D%0Aclass%20Baz%20extends%20Bar%20%7B%0D%0A%0D%0A%20%20%20%20test()%20%7B%0D%0A%20%20%20%20%20%20%20%20this.bar%20%3D%20'barbar'%20%2F%2F%20should%20work!%0D%0A%20%20%20%20%20%20%20%20this.foo%20%3D%20'foofoo'%20%2F%2F%20should%20work!%0D%0A%20%20%20%20%7D%0D%0A%0D%0A%7D%0D%0A%0D%0Aconst%20f%3A%20Foo%20%3D%20new%20Bar())

Is there a way to do this currently, that we may have missed? (cc: @justinfagnani)

Checklist

My suggestion meets these guidelines:

sandersn commented 5 years ago

@trusktr For your first example, you just need a couple of type annotations to get rid of errors:

type Ctor<T> = { new(): T }
function FooMixin(Base: Ctor<{}>) {
    return class Foo extends Base {
        foo = 'foo'
    }
}
function BarMixin(Base: Ctor<{}>) {
    return class Bar extends FooMixin(Base) {
        test() {
            console.log(this.foo) // this.foo is obviously inherited from FooMixin!
                           // ^--- This shoud not be an error!
        }
    }
}

For your second example, you can't use typeof HTMLElement directly because its constructor is too restrictive to fit Typescript's mixin pattern. But you can just create a similar type, which typeof HTMLElement happens to be assignable to:

function FooMixin<T extends new (...args:any[]) => HTMLElement>(Base: T) {
    return class Foo extends Base {
        foo = 'foo'
        test() {
            this.setAttribute('foo', 'bar')
        }
    }
}

I haven't read your full example or @dragomirtitian's suggested workaround on SO. Let me do that now.

trusktr commented 5 years ago

I don't quite understand. Why does your first example work, and mine (with the Constructor type, doesn't? It's not immediately clear. Seems as if I'm specifying basically the same thing.

trusktr commented 5 years ago

How come when I change your example from

type Ctor<T> = { new(): T }
function FooMixin(Base: Ctor<{}>) {
    return class Foo extends Base {
        foo = 'foo'
    }
}

to

type Ctor<T> = { new(): T }
function FooMixin<T extends Ctor<{}>>(Base: T) {
    return class Foo extends Base { // ERROR, Type 'T' is not a constructor function type.
        foo = 'foo'
    }
}

it no longer works?

sandersn commented 5 years ago

There's a hard-coded requirement the mixin's type must be an object type, which Ctor<{}> is, but T extends Ctor<{}> is not -- it's a type parameter.

Using Ctor<{}> directly may be enough for you; T extends Ctor<{}> is treated like Ctor<{}> inside FooMixin, and when you call FooMixin, anything assignable to T extends Ctor<{}> is also assignable to Ctor<{}>.

The only reason you'd need a type parameter is to make other parameters of FooMixin use type T. For example, if you wanted to mixin two things, you could make those two have the exact same type:function FooMixin<T extends Ctor<{}>>(Base1: T, Base2: T). That's weird! Although, there might be more believable examples.

trusktr commented 5 years ago

@sandersn I tried your recommendation with the HTMLElement, but seems I can't do it with a default arg. For example:

type Constructor<T = any, A extends any[] = any[]> = new (...a: A) => T

function WithUpdateMixin<T extends Constructor<HTMLElement>>(Base: T = HTMLElement) {

but with the = HTMLElement default arg it says:

Type '{ new (): HTMLElement; prototype: HTMLElement; }' is not assignable to type 'T'.
  '{ new (): HTMLElement; prototype: HTMLElement; }' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint 'Constructor<HTMLElement, any[]>'. 
ts(2322)

It doesn't make sense to me. How does HTMLElement not fit the constraint?

trusktr commented 5 years ago

Here's another example of better support needed for mixins. In my previous comment, WithUpdateMixin is for Web Components. At minimum, the Base needs to be one of the builtin element classes like HTMLElement, HTMLDivElement, etc.

But the Base class could also be a Custom Element that extends from one of those builtin classes.

So in my code (converting from JavaScript), I have the following conditional, in case the Base class has an attributeChangedCallback method (if it is a Custom Element):

            if (super.attributeChangedCallback) {
                super.attributeChangedCallback(name, oldValue, newValue)
            }

but TypeScript gives the error:

Property 'attributeChangedCallback' does not exist on type 'HTMLElement'. ts(2339)

Obviously it doesn't exist on HTMLElement.

I also tried

(super as any).attributeChangedCallback

but it is invalid syntax for TS.

Same problem with

            if (super.connectedCallback) {
                super.connectedCallback()
            }

etc.

I also tried putting this: any in the method signature which has the above conditionals, but no luck.

How can we make this work?

trusktr commented 5 years ago

Alright, I'm trying to work around the problem by forcing a cast, but it isn't making sense. I'm trying:

type PossibleCustomElement<T = HTMLElement> = T & {
    connectedCallback?(): void
    disconnectedCallback?(): void
    adoptedCallback?(): void
    attributeChangedCallback?(name: string, oldVal: string | null, newVal: string | null): void
}

function WithUpdateMixin<T extends Constructor<HTMLElement>>(Base: T = HTMLElement as any) {
    return class WithUpdate extends (Base as PossibleCustomElement<T>) {
                                               // ^--- HERE

and it says

Type 'PossibleCustomElement<T>' is not a constructor function type. ts(2507)

Clearly T is constrained to T extends Constructor<HTMLElement>, so it seems that therefore it is a constructor and I shouldn't get a type error.

Is this simply beyond what is implemented in TypeScript? Does TypeScript need a new feature here?

EDIT: Okay, I see PossibleCustomElement is the instance type. So I got a little closer, but no cigar:

type PossibleCustomElement<T extends HTMLElement> = T & {
    connectedCallback?(): void
    disconnectedCallback?(): void
    adoptedCallback?(): void
    attributeChangedCallback?(name: string, oldVal: string | null, newVal: string | null): void
}

type PossibleCustomElementConstructor<T extends HTMLElement> = Constructor<PossibleCustomElement<T>>

function WithUpdateMixin<T extends Constructor<HTMLElement>>(Base: T = HTMLElement as any) {
    return class WithUpdate extends ((Base as unknown) as PossibleCustomElementConstructor<T>) {

and now the error is:

Base constructor return type 'PossibleCustomElement<T>' is not an object type or intersection of object types with statically known members.ts(2509)

So now my question seems to be valid: Should it be able to understand that an instance of PossibleCustomElementConstructor<T> (which is constrained to extend from the HTMLElement constructor) should at least have the methods and properties from from HTMLElement and PossibleCustomElement?

trusktr commented 5 years ago

Ah! I figured it out.

The problem was: I was intuitively thinking that the constraint T extends Constructor<HTMLElement> would make ((Base as unknown) as PossibleCustomElementConstructor<T>) in my previous comment work, because I had assumed the type system would understand the constraint and implicitly limit the application of PossibleCustomElementConstructor<T> to PossibleCustomElementConstructor<HTMLElement>. Turns out, I was being too smart for the type system! 🤓 (Maybe this is a new feature request then).

So, I figured out that if I explicitly specify the constrained type, instead of intuitively passing T along, then it works, like follows:

type PossibleCustomElement<T extends HTMLElement> = T & {
    connectedCallback?(): void
    disconnectedCallback?(): void
    adoptedCallback?(): void
    attributeChangedCallback?(name: string, oldVal: string | null, newVal: string | null): void
}

type PossibleCustomElementConstructor<T extends HTMLElement> = Constructor<PossibleCustomElement<T>>
type HTMLElementConstructor = Constructor<HTMLElement>

function WithUpdateMixin<T extends HTMLElementConstructor>(Base: T = HTMLElement as any) {
    return class WithUpdate extends ((Base as unknown) as PossibleCustomElementConstructor<HTMLElement>) {

        connectedCallback() {
            if (super.connectedCallback) {  // ----- IT WORKS!
                super.connectedCallback()
            }

            // ...
        }

        attributeChangedCallback(name: string, oldValue: string | null, newValue: string | null) {
            if (super.attributeChangedCallback) { // ----- IT WORKS!
                super.attributeChangedCallback(name, oldValue, newValue)
            }

            // ...
        }
    }
}

And now that makes sense: I know that the type is at least HTMLElement, so if I explicitly state that in the extends part of the mixin class, then I know that's what I will at least get (as far as types that I can use inside of the WithUpdate class.

I think it would make sense for TypeScript to infer what I was trying to do by passing T along, instead of having to specify HTMLElement in more than one place; it would make things more DRY.

But I still don't like the as unknown cast; I feel like that shouldn't be there. (Although the combination of the words "unknown" and "PossiblyCustomElement" ironically fit together because we don't know what Base will be exactly).

trusktr commented 5 years ago

Now I need to figure out if the main thing in the original post is possible: making mixins composed of mixins...

The issue I forsee is that WithUpdate could be mixed with other mixins that don't necessarily extend from HTMLElement, so that later the composed mixin could finally be mixed with a subclass that does extend HTMLElement.

trusktr commented 5 years ago

Thanks for your hints about T extends Ctor<{}>. That helped a lot. I've gotten my mixins working (after many permutations of tinkering to understand how the type checker works), however it still requires a cast because the base class "is not a constructor function type". I found it easier to write my own Constructor helper, with default T = object, which is what I need in most cases.

Basically, here's what a simplified version looks like:

type Constructor<T = object, A extends any[] = any[]> = new (...a: A) => T

function FooMixin<T extends Constructor>(Base: T) { 

    class Foo extends Base { 
        foo = 'foo'
    }

    return Foo as typeof Foo & T
}

function BarMixin<T extends Constructor>(Base: T) { 

    class Bar extends FooMixin(Base as unknown as Constructor) { 
        test() { 
            console.log(this.foo)
        }
    }

    return Bar as typeof Bar & T

}

playground

The as Constructor cast doesn't seem intuitive, but needed when using T in order to return the mixin type combined with the passed-in constructor type.

RyanCavanaugh commented 5 years ago

@trusktr What further action is required here in your judgment?

trusktr commented 4 years ago

@RyanCavanaugh I think that Mixins need to be made easier. Taking the previous comment's example,

It gets more complicated when adding more mixins into the mix, when the constraint on T is more specific, when needing to include static properties in the return type, etc.

For a look at how complicated and brittle the mixin boilerplates can get, see for example infamous/src/core/Node.ts (you should be able to clone, npm install, and then see all the types in VS Code if there hasn't been any breaking changes in typescript, it's been a while).

trusktr commented 4 years ago

I closed https://github.com/microsoft/TypeScript/issues/32004 as a duplicate of this one.

trusktr commented 4 years ago

It'd be great if mixins in TypeScript were easier. Here's another playground example, and the code:

type AnyCtor = new (...a: any[]) => any

function Foo<T extends AnyCtor>(Base: T) {
    return class Foo extends Base {
        foo() {}
    }
}
function Bar<T extends AnyCtor>(Base: T) {
    return class Bar extends Base {
        bar() {}
    }
}
function One<T extends AnyCtor>(Base: T) {
    return class One extends Base {
        one() {}
    }
}
function Two<T extends AnyCtor>(Base: T) {
    return class Two extends Base {
        two() {}
    }
}
function Three<T extends AnyCtor>(Base: T) {
    return class Three extends One(Two(Base)) {
        three() {}
    }
}

class MyClass extends Three(Foo(Bar(Object))) {
    test() {
        this.foo()
        this.bar()
        // @ts-expect-error
        this.one() // Uh oh! This is type 'any', and there is no error despite noImplicitAny being set to true!
        // @ts-expect-error
        this.two() // Uh oh! This is type 'any', and there is no error despite noImplicitAny being set to true!
        this.three()
        console.log('no errors')
    }
}

const m = new MyClass()
m.test()

In particular, notice that this.one and this.two are type any inside the test() method.

ShaMan123 commented 1 year ago

+1 @trusktr thanks so much for posting all your tryouts! I've spent too long on this (I am embarrassed to say, around 6 hours), this thread was what saved me. I encountered a complex case of generic classes that are mixed as base classes. TS went mad and so did I.

jahorton commented 8 months ago

I've been experimenting a bit with mixins lately and believe I've run into a related case. As the example code might be a bit large due to the setup, a brief explanation:

I'm looking to create mixins for a class intended for serialization and deserialization in certain cases. Serialization of mixin data isn't too hard of an ask, but the deserialization process for a mixin-enhanced version of the class - that seems to be the hard part, as I want to do it through static methods to preserve proper encapsulation of instances, avoiding exposure of a potentially-incomplete instance. (This can be especially noteworthy for types that are designed to act immutable.)

To be explicit, I consider a requirement of preknowledge of the set of mixins involved to be reasonable. (No mixin-inference during deserialization.) I'm looking to deserialize from the final, fully mixed-in class.

I believe I've actually found a way forward on this, though there's one last caveat I wasn't able to completely polish. I've kept a bit of noise in case the variations on things I attempted leading up to this could be enlightening.

Playground example

// Based on https://stackoverflow.com/a/43723730

interface ConfigConstructor {
  restoreFromJSON: (obj: any) => any;
  _loadFromJSON: (target: ConfigImpl, obj: any) => ConfigImpl;
  new (a: string, b: string /*...args: any[]*/): ConfigImpl;
}

class ConfigImpl {
  // Offline, I can make these private with exposing `get`-ter properties...
  // ...but it seems the TS Playground isn't a fan of having them private.
  public name: string;
  public title: string;

  constructor(name?: string, title?: string) {
    // // Supports the `restoreFromJSON` method if not using optional params.
    // if(arguments.length == 0) {
    //   this._name = '';
    //   this._title = '';
    //   return;
    // }

    this.name = name ?? '';
    this.title = title ?? '';
  }

  static restoreFromJSON = function (obj: any) {
    const restored = new ConfigImpl('', '');
    return ConfigImpl._loadFromJSON(restored, obj);
  }

  static _loadFromJSON = function (target: ConfigImpl, obj: any) {
    target.name  = obj.name;
    target.title = obj.title;

    return target;
  }
}

type AnyConstructor<A = object> = new (...input: any[]) => A
const Config: AnyConstructor<ConfigImpl> & ConfigConstructor = ConfigImpl;

Config.restoreFromJSON;
const dummy = new Config("dummy", "object");
dummy.name;
dummy.title;

// Defining the mixin is... not the easiest, but this is the only ugly part.  I think.
// function IntermediateMixin<T extends AnyConstructor<typeof Config> & Omit<typeof Config, 'new'>>(Base: T) { // falls over. :(
function IntermediateMixin<T extends AnyConstructor<ConfigImpl> & Omit<typeof Config, 'new'>>(Base: T) { // works!
// function IntermediateMixin<T extends AnyConstructor<ConfigImpl> & Omit<ConfigConstructor, 'new'>>(Base: T) { // works!
  return class MixinCore extends Base {
    public counter: number = 0;

    // Utilizes the fact that constructors themselves inherit from each other in JS/TS.
    // We call the version on the full mixin...
    static restoreFromJSON(obj: any): MixinCore {
      // ******************************************************
      // * Key detail:  Assumes an empty constructor is fine. *
      // ******************************************************
      return MixinCore._loadFromJSON(new MixinCore(), obj);
    }

    // Which, in turn, calls each constituent mixin (+ the base) version of the deserialization static.
    static _loadFromJSON(target: MixinCore, obj: any): MixinCore {
      // let the base load its stuff.
      Base._loadFromJSON(target, obj);

      // then load our stuff
      target.counter = obj.counter;

      // then return (in case another mixin has its own loading to do, too.)
      return target;
    };

    public counterOver10(): boolean {
      return this.counter > 10;
    }
  }
}

const funkyMixin = IntermediateMixin(Config);
const funkyDummy = new funkyMixin("funky", "dummy");
// const funkyDummy2 = new funkyMixin('a');  // errors on the extra param.
funkyMixin.restoreFromJSON;
// funkyDummy.restoreFromJSON; // error; is not an instance field/method

const reconstituted = funkyMixin.restoreFromJSON({
  name: "foo",
  title: "bar",
  counter: 13
});

console.log(JSON.stringify(reconstituted, null, 2));
reconstituted.counter;
if(reconstituted.counterOver10()) {
  console.log("`reconstituted`: success - counter is over 10");
}

// ---------

// function ExtraMixin<T extends AnyConstructor<ConfigImpl> & Omit<typeof Config, 'new'>>(Base: T) { // works!
function ExtraMixin<T extends AnyConstructor<ConfigImpl> & Omit<ConfigConstructor, 'new'>>(Base: T) {
  return class ExMixinCore extends Base {
    public flag: boolean = true;

    // Utilizes the fact that constructors themselves inherit from each other in JS/TS.
    // We call the version on the full mixin...
    static restoreFromJSON(obj: any): ExMixinCore {
      // Key detail:  Assumes an empty constructor is fine.
      return ExMixinCore._loadFromJSON(new ExMixinCore(), obj);
    }

    // Which, in turn, calls each constituent mixin (+ the base) version of the deserialization static.
    static _loadFromJSON(target: ExMixinCore, obj: any): ExMixinCore {
      // let the base load its stuff.
      Base._loadFromJSON(target, obj);

      // then load our stuff
      target.flag = obj.flag;

      // then return (in case another mixin has its own loading to do, too.)
      return target;
    };
  }
}

// const stackedMixin = ExtraMixin(MixinWithStatics(funkyMixin));
// const stackedMixin = ExtraMixin<typeof funkyMixin>(MixinWithStatics(Config));
const stackedMixin = ExtraMixin(IntermediateMixin(Config));
const directBuilt = new stackedMixin('foo', 'bar');
directBuilt.counter;
directBuilt.flag;

      // ******************************************************
      // * Setup complete; now for limitations I discovered   *
      // ******************************************************

const reconstituted2 = stackedMixin.restoreFromJSON({
  name: "foo",
  title: "bar",
  counter: 13,
  flag: false
}); // if this replaces the following line, compilation errors arise on lines noted below.
// }) as InstanceType<typeof stackedMixin>;  // Uncomment out the section to the left to replace the call's end, and things work!

let swappable = directBuilt;
// swappable = reconstituted2; // bombs - claims the intermediate mixin's properties don't exist.
let reverseSwappable = reconstituted2;
reverseSwappable = directBuilt; // passes swimmingly; no complaints here.

// Works regardless of the `reconstituted2` cast.
console.log(JSON.stringify(reconstituted2, null, 2));
reconstituted2.flag;

// Why doesn't TS recognize that these are available without the most recent cast
// (when assigning to `reconstituted2`)?
reconstituted2.counter;  // error - `counter` undefined
if(reconstituted2.counterOver10() && !reconstituted2.flag) {  // error - `counterOver10` undefined
  console.log("`reconstituted`: success - counter is over 10 & flag is false");
}

// Aha!  The deserialization pattern isn't quite getting resolved in full.
// But... shouldn't it resolve to the exact same type?
const cast = reconstituted2 as InstanceType<typeof stackedMixin>; // JUST a cast.
cast.counter;
cast.counterOver10;
// Works, but is redundant with the prior if-block.
// if(cast.counterOver10() && !cast.flag) {
//   console.log("`cast`: success - counter is over 10 & flag is false");
// }

I managed to get the mixins working with references to static methods declared by the base class; they even redirect to variants defined on intermediate mixins! The main issue - the final return value from the deserialization method has incomplete type inference in a manner very much in line with other cases I noticed on this issue.

Related highlight from my example:

function ExtraMixin<T extends AnyConstructor<ConfigImpl> & Omit<ConfigConstructor, 'new'>>(Base: T) {
  return class ExMixinCore extends Base {
    public flag: boolean = true;

    // Utilizes the fact that constructors themselves inherit from each other in JS/TS.
    // We call the version on the full mixin...
    static restoreFromJSON(obj: any): ExMixinCore {
      // Key detail:  Assumes an empty constructor is fine.
      return ExMixinCore._loadFromJSON(new ExMixinCore(), obj);
    }

Note: the static method is returning the same type as the mixin, based on the generic parameter <T extends ...>, as noted to be a possible issue in the first comment of this issue. I'm not sure it's easy or wise to eliminate it though - I fully want the two mixins to be orthogonal, and this allows the intermediate properties to flow through for normally-constructed instances.

The other main link that led me to this issue: https://github.com/microsoft/TypeScript/issues/32080#issuecomment-667800147. I noticed that that comment's example one() and two() were absent due to mixin composition - these were composited within the Three mixin. In my example code above, IntermediateMixin is (obviously) the composited mixin, and it's the one with the missing property and method. In particular, in this case, the static method's return type returns an object whose typing refers to that of another mixin... which is the same type as the static method's class - but TS is unable to infer this at present.

Fortunately, it's possible to work around this oddity with a cast, but it does make the deserialization process a bit more verbose than desired.

jahorton commented 8 months ago

What I noticed there gave me an idea: what if the ConfigConstructor class there were generic, with a type parameter that adapts to the expected return type for its constructor & static methods?

Well... it was worth a shot; we actually get more errors this way. But... those might be enlightening?

// Was `ConfigConstructor`, but this is likely a better / more abstract name for what it seeks to represent.
interface ConstructorForRestorable<Type> {
  restoreFromJSON: (obj: any) => Type;
  _loadFromJSON: (target: Type, obj: any) => Type;
  new (a: string, b: string /*...args: any[]*/): Type;
}

// ...

function IntermediateMixin<T extends AnyConstructor<ConfigImpl> & Omit<ConstructorForRestorable<T>, 'new'>>(Base: T) { // works!
  return class MixinCore extends Base {
    public counter: number = 0;

    // Utilizes the fact that constructors themselves inherit from each other in JS/TS.
    // We call the version on the full mixin...
    static restoreFromJSON(obj: any): MixinCore {
      // ******************************************************
      // * Key detail:  Assumes an empty constructor is fine. *
      // ******************************************************
      return MixinCore._loadFromJSON(new MixinCore(), obj);
    }

    // Which, in turn, calls each constituent mixin (+ the base) version of the deserialization static.
    static _loadFromJSON(target: MixinCore, obj: any): MixinCore {
      // let the base load its stuff.
      Base._loadFromJSON(target, obj);  // error squiggles appear here.

      // then load our stuff
      target.counter = obj.counter;

      // then return (in case another mixin has its own loading to do, too.)
      return target;
    };

The error from the noted line:

(parameter) target: MixinCore Argument of type 'MixinCore' is not assignable to parameter of type 'T'. 'T' could be instantiated with an arbitrary type which could be unrelated to 'MixinCore'.ts(2345)

Note: class MixinCore extends Base, with the mixin's parameter defined as Base: T. There's a very clear relation here - class MixinCore extends T - but the inference engine isn't connecting the dots.

This is likely due to the generic-param T, as opposed to the parameter Base of type T. At least, that's my personal guess.

This error and related behaviors also appear to prevent actually applying the mixin; the genericized class that props up the static method pseudo-inheritance is thus not viable.

Error when attempting to use the mixin with this code variant:

Argument of type 'AnyConstructor & ConstructorForRestorable' is not assignable to parameter of type 'AnyConstructor & Omit<ConstructorForRestorable<AnyConstructor & ConstructorForRestorable>, "new">'. Type 'AnyConstructor & ConstructorForRestorable' is not assignable to type 'Omit<ConstructorForRestorable<AnyConstructor & ConstructorForRestorable>, "new">'. The types returned by 'restoreFromJSON(...)' are incompatible between these types. Type 'ConfigImpl' is not assignable to type 'AnyConstructor & ConstructorForRestorable'. Type 'ConfigImpl' is not assignable to type 'AnyConstructor'. Type 'ConfigImpl' provides no match for the signature 'new (...input: any[]): ConfigImpl'.ts(2345)

That final line is a fun read. Essentially, "the type doesn't provide its own constructor"? Perhaps that's from the constructor-signature shenanigans I had to pull to meet the base mixin constructor requirements... except no, it persists even if that's the exact signature for the real constructor.

boconnell commented 7 months ago

@trusktr I think the problem with your initial example and the example in https://github.com/microsoft/TypeScript/issues/32080#issuecomment-667800147 doesn't explicitly have to do with mixins: It's that you are using any as the instance type of your Constructor type instead of object. In both case, if you change type Constructor = new (...a: any[]) => any to type Constructor = new (...a: any[]) => object, everything works as expected. TS playground 1, TS playground 2

When Typescript merges the instance types of these constructors, everything is just getting collapsed into any, more because of how any behaves than anything specific to constructors.