Generating type definitions for mixin classes with protected members

[fr0]

TypeScript Version: 2.4.2

Code:

I'm using mixins as described by: #13743

export type Constructor<T> = new(...args: any[]) => T;
export function Unsubscriber<T extends Constructor<{}>>(Base: T)  {
  class Unsubscriber extends Base implements OnDestroy {
    protected unsubscribe: Subject<void> = new Subject();

    ngOnDestroy() {
      this.unsubscribe.next();
      this.unsubscribe.complete();
    }
  }
  return Unsubscriber;
}

If I compile this code with "declaration": true to get type definitions for my library, I get the following error:

ERROR in (truncated)/mixins.ts (...): Return type of exported function has or is using private name 'Unsubscriber'.

One solution is to add an interface...

export interface IUnsubscriber extends OnDestroy {
  unsubscribe: Subject<void>;
}

...and have my mixin function have a return type of Constructor<IUnsubscriber>. This works, but it forces me to make the properties/methods exposed by my mixin be public even in cases where I want them to be protected.

Short of adding protected members to interfaces (which I'm not sure is the right thing to do), this seems to be a limitation of the currently supported mixin strategy.

[octogonz]

BTW I found that you can work around this limitation to some extent by using ECMAScript symbols, which provide a non-TypeScript way to hide a member:

export type Constructor<T> = new(...args: any[]) => T;

const unsubscribe: unique symbol = Symbol('Unsubscriber.unsubscribe');

export function Unsubscriber<T extends Constructor<{}>>(Base: T)  {
  class Unsubscriber extends Base implements OnDestroy {
    public [unsubscribe]: Subject<void> = new Subject();

    ngOnDestroy() {
      this[unsubscribe].next();
      this[unsubscribe].complete();
    }
  }
  return Unsubscriber;
}

IntelliSense will only show the unsubscribe member to callers who have access to the unsubscribe symbol.

[hueyhe]

It bothers me too. Seems that this issue was moved into future milestone. No plan to solve it in the near future?

[octogonz]

BTW I recently came up with an alternate way to represent mixins in TypeScript. Consider this example:

// Helper that copies properties from base's prototype
// to child's prototype
function extend(child: any, base: any): void {
  for (var property in base) {
    var descriptor = Object.getOwnPropertyDescriptor(base, property) || { value: base[property] };
    Object.defineProperty(child, property, descriptor);
  }
}

// The mixin base classes
class Base1 {
    constructor() {
        console.log('Base1.constructor() called')
    }
    public method1(): void {
        console.log('method1() called')
    }
}

class Base2 {
    constructor() {
        console.log('Base2.constructor() called')
    }
    public method2(): void {
        console.log('method2() called')
    }
}

class Base3 {
    constructor() {
        console.log('Base3.constructor() called')
    }
    private _prop3 = 123;
    public get prop3(): number {
        return this._prop3;
    }
}

// Collect the base classes into an intermediary "Mixin" class
type Mixin = Base1 & Base2 & Base3;
const Mixin: { new(): Mixin } = class { 
    constructor() {
        Base1.call(this);
        Base2.call(this);
        Base3.call(this);
    }
} as any;
extend(Mixin.prototype, Base1.prototype);
extend(Mixin.prototype, Base2.prototype);
extend(Mixin.prototype, Base3.prototype);

// Extend from the "Mixin" class
class Child extends Mixin {
    public method4(): void {
        console.log('method4() called')
    }    
}

const child = new Child();

child.method1();
child.method2();
console.log('prop3 = ' + child.prop3);
child.method4();

The behavior is probably unsound if the base classes happen to have members with the same name. But this issue could be detected in the extend() function.

There are some interesting advantages:

• The base classes don't need to be represented specially; they are ordinary TypeScript classes
• You can use private members without any trouble
• The generated .d.ts file is simple and understandable

Not sure I'd recommend to do this in real code, but it's interesting to contrast with the class-expression approach to mixins.

[Elvynia]

It bothers me too. Seems that this issue was moved into future milestone. No plan to solve it in the near future?

Well me too !

@RyanCavanaugh Any update on when this could be fixed ? It's set on the "Future" backlog but as it's pretty huge, could you add some precision ?

[web-padawan]

Here is my research regarding possible workarounds for TS mixins with protected methods.

Overview

These are 3 approaches that can be used for writing mixins that need to expose protected methods.
They all allow subclasses extending such mixins to call these methods using super.

1. Symbol approach

• Not using protected methods in the mixins

• Using Symbol as recommended by Elix

• Example: see above comment Generating type definitions for mixin classes with protected members #17744 (comment)

Pros

• Methods are truly protected (can't be called from outside, e.g. by third party code)

• Methods are "public" in terms of TS and can be annotated on the interface

• Explicit exports potentially help with refactoring of the consumers' subclasses

• Methods are excluded from the DevTools console's auto-complete list (good for user)

• Avoid potential name conflicts if user extends a class with their own methods

Cons

• Consumers have to import symbols if they need to override protected methods

• Jump to definition is not ideal (jumps to the place where Symbol() is created)

2. Dumb class approach

• Using protected methods in the mixins

• Using abstract class SomeMixinClass to "annotate" them

• Example: [ts] 'private' modifier cannot appear on a type member. #25163 (comment)

Pros

• No need to import anything (except the mixin itself) for the consumer component

• Methods are easy to access in the DevTools console (good for maintainer)

Cons

• Both interface (for public API) and abstract class (for protected API) are needed

• The abstract class ends up in the JS output (as empty class with no methods)

• The mixin function needs to confusingly accept the argument with a type cast (instead of the actual expected class) in order to pick up the annotated protected methods from it:

// with dumb class: confusing
<T extends Constructor<SlottedItemsClass>>(base: T)

// without dumb class: clean
<T extends Constructor<LitElement>>(base: T)

• Need for extra super check: super._focus && super._focus() - this is needed because the abstract classes mark methods using _focus?(), i.e. "possibly undefined"

• Jump to definition from the subclass goes to a dumb class

3. Public methods

• Keep methods public, add them on the interface

• Jump to definition works as expected

• Example: Make members private again hypermedia-app/Alcaeus#33

Pros

• No extra code needed (symbols / dumb classes)

Cons

• TS users would get protected methods listed in the code completion

• Messing up public and protected API might need tweaks for tools

Summary

Personally, I like symbols approach more because it's cleaner and bulletproof. The "jump to definition" inconvenience is a drawback that is more of a personal preference.

On the other hand, the dumb class approach has its benefits: once the issue is resolved, we can get rid of those dumb classes and potentially keep the methods themselves unchanged.

---

@weswigham according to your comment at #17293 (comment), what do you have on your mind to tackle these issues? Is there a plan, or does this still need a concrete proposal that you mentioned?

[weswigham]

Is there a plan, or does this still need a concrete proposal that you mentioned?

Still needs a proposal.

[trusktr]

@fr0 Also worth noting #35822

[justinfagnani]

We hit this issue again when we tried to factor out a base class with private and protected members into a mixin.

Because we've already provided protected methods to subcasses of our base class, we really needed to keep existing protected members protected. If the members became public, then subclasses would get errors on trying to override public members with protected.

The workarounds we've had to do to satisfy the compiler are pretty onerous. We've created a fake class outside of the mixin and cast the mixin function to return an intersection of the argument and that class. Static require another fake class. Then we need to get our build system to remove the fake class. I'm not sure this approach solved every problem yet.

@weswigham

Still needs a proposal.

Is there anything anyone not on the TS team can do to help here?

[weswigham]

Well, we discussed it at our last design meeting as future work beyond abstract constructor types, however some team members think allowing a class-expression-like-thing in a type position (and having that refer to the static shape of the declared class) would be confusing. IMO, it makes sense since the shape stored by

type A = class {}

would be the same as the type of the const in

const A = class {}

But that's only my point of view. The disagreement within the team means we're going to need to see both a serious proposal and serious justification for why it's needed and how it makes sense before we're going to get close to agreement.

[jogibear9988]

@weswigham I think a list of why it is needed is in this issue: #35822
The list in the Description.

[hjkcai]

I come up with a (maybe) visually better solution based on The Dumb Class Approach.

Pros:

• Easier to understand by hiding the ugly type details

Cons:

• Still need to define two classes
• Unsound: no guarantee that the mixin class implements the dumb class correctly
• A little runtime overhead: uses a function createMixin to make TypeScript infer types

export type ConstructorOf<T> = new (...args: unknown[]) => T

export type MixinFactory<T, TBase> = (Base: ConstructorOf<TBase>) => ConstructorOf<T>
export interface MixinOf<T> {
  (): ConstructorOf<T>
  <U = {}>(Base?: ConstructorOf<U>): ConstructorOf<T & U>
}

export function createMixin<TBase = {}, T = TBase>(factory: MixinFactory<T, TBase>): MixinOf<T & TBase> {
  return ((Base = Object) => factory(Base as any)) as any
}

export type MixinType<T extends MixinOf<unknown>> = T extends MixinOf<infer U> ? U : never

// Case 1. define a normal mixin (with no type parameter)
export type FooMixin = MixinType<typeof FooMixin>
export const FooMixin = createMixin(Base => class FooMixin extends Base {
  foo() {}
})

// Case 2. define a mixin with protected/private fields (with type parameter)
declare class BarMixinBase {
  protected bar(): void
}

export type BarMixin = MixinType<typeof BarMixin>
export const BarMixin = createMixin<BarMixinBase>(Base => class BarMixin extends Base {
  protected bar() {}
})

// Usage
export class MyClass extends FooMixin(BarMixin()) {
  test() {
    this.foo()
    this.bar()
  }
}

Hope the TS team to eventually solve the issue.