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

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

Open
5 tasks done
trusktr opened this issue Jun 25, 2019 · 18 comments
Open
5 tasks done

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

trusktr opened this issue Jun 25, 2019 · 18 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@trusktr
Copy link
Contributor

trusktr commented Jun 25, 2019

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

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

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

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@sandersn
Copy link
Member

@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.

@sandersn sandersn added the Needs Investigation This issue needs a team member to investigate its status. label Jun 26, 2019
@trusktr
Copy link
Contributor Author

trusktr commented Jun 26, 2019

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
Copy link
Contributor Author

trusktr commented Jun 27, 2019

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
Copy link
Member

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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

@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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

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
Copy link
Contributor Author

trusktr commented Jun 30, 2019

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
Copy link
Member

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

@RyanCavanaugh RyanCavanaugh added Needs More Info The issue still hasn't been fully clarified and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 22, 2019
@trusktr
Copy link
Contributor Author

trusktr commented Nov 17, 2019

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

  • it shouldn't require casting Base as unknown as Constructor
  • it shouldn't require casting return Bar as typeof Bar & T

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
Copy link
Contributor Author

trusktr commented Dec 28, 2019

I closed #32004 as a duplicate of this one.

@trusktr
Copy link
Contributor Author

trusktr commented Aug 3, 2020

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
Copy link

ShaMan123 commented Dec 6, 2022

+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.

ShaMan123 added a commit to fabricjs/fabric.js that referenced this issue Dec 6, 2022
OMG what a nightmare
A time sink

microsoft/TypeScript#32080
@jahorton
Copy link

jahorton commented Dec 30, 2023

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: #32080 (comment). 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
Copy link

jahorton commented Dec 30, 2023

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
Copy link

@trusktr I think the problem with your initial example and the example in #32080 (comment) 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.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed Needs More Info The issue still hasn't been fully clarified labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants