tannerntannern / ts-mixer

A small TypeScript library that provides tolerable Mixin functionality.
MIT License
384 stars 27 forks source link

Abstract mixins #13

Closed KrisBraun closed 4 years ago

KrisBraun commented 4 years ago

I'd like to use abstract classes for mixins to impose requirement on the classes that use them, like this:

import { Mixin } from 'ts-mixer'

class Parent {
}

abstract class TestMixin {
  public get bar() {
    return this.foo
  }

  protected abstract get foo(): string
}

class Test extends Mixin(Parent, TestMixin) {
  protected get foo(): string {
    return 'foo'
  }
}

This produces an error on the Mixin line:

Cannot assign an abstract constructor type to a non-abstract constructor type. TS2345

Could something like this be possible?

tannerntannern commented 4 years ago

Hi Kris, thanks for opening this issue. As it stands currently, this would be difficult to support. Before I go down any rabbit holes, can I get some more context as to why you want to do this?

KrisBraun commented 4 years ago

Thanks for the quick response (and creating such a useful library)!

I have models with an underlying datastore, and I'd like to create mixins that are agnostic to those details. For example, a mixin that provides item ordering operations may need to know whether the model object is in a completed state. The cleanest way to do this seems to be for the mixin to have an abstract completed function which is implemented by the model.

tannerntannern commented 4 years ago

Hmm... and I assume the reason you didn't go with an interface is because interfaces can't have protected properties?

KrisBraun commented 4 years ago

An interface instead of a mixin? I'm using the latter since the non-abstract methods have implementations.

My workaround for now is to make the mixin class concrete and provide implementations of the required methods that throw errors. Effectively, I miss out on static checking that all required methods are implemented, but get the rest of the what I want.

tannerntannern commented 4 years ago

Yeah, my suggestion was to move the abstract methods into a separate interface from the mixin, so the mixin class doesn't have to be abstract. Then you could do something like:

class Test extends Mixin(Parent, TestMixin) implements ITestMixin { /* ... */ }

However, I now see that this may not work for you since the methods are interdependent. 🤔

tannerntannern commented 4 years ago

I do think mixing abstract classes could be useful. However, I'm not sure how to handle the "abstractness" of the mixed result. Say you mix a concrete and an abstract class: what should the mixed class be? Abstract? What if the concrete class implements all the required methods? Should it still be abstract or concrete?

tannerntannern commented 4 years ago

Hi again Kris. I was messing around with a workaround for this problem and I came up with this.

Do let me know what you think of this approach! Might be a suitable workaround for now.

KrisBraun commented 4 years ago

@tannerntannern, that is amazing! Works perfectly and completely handles my use case.

Out of curiosity, how did you know or discover abstract methods could be used outside abstract classes, with differing behaviour?

In any case, thank you very much for coming back to this. I find this library very helpful.

tannerntannern commented 4 years ago

@KrisBraun, glad to hear it!

Regarding your question, the discovery was somewhat of an accident. The problem that Mixin(...) has with abstract classes is that it expects each argument to conform to the type {new(): InstanceType} (simplified). But of course, abstract classes can't fit this type because they, by definition, can't be constructed. Out of curiosity, I wanted to see what would happen if TypeScript was "tricked" into thinking the class was concrete, even though it had abstract methods. Turns out it worked great! However, it really is an abuse of the type checker so I don't know if this will continue to work in future TS versions.

Glad you find the library useful. I'm working on a new major version that will be released fairly soon, so stay tuned!