Closure-compiler: Support expression in extends clauses

[justinfagnani]

Demo

Classes can extend arbitrary expressions, but Closure is throwing a warning on an unknown type.

const Mixin = (superclass) => class extends superclass {
  // ...
}

Note, the warning from the compiler service at least doesn't have a line number:

JSC_TYPE_PARSE_ERROR: Bad type annotation. Unknown type superclass at line -1 beyond character 1

[justinfagnani]

/cc @ChadKillingsworth @rictic

[ChadKillingsworth]

For background: http://justinfagnani.com/2015/12/21/real-mixins-with-javascript-classes/
Planned for use in Polymer 2.

[justinfagnani]

I was going to file separate issues for each aspect of this pattern that fails, but maybe it's easier if they are together.

Another problem is that I can't type a mixin _application_ as extending a specific superclass and adding it's own properties:

Given a normal class Base, and a mixin function Mixin that returns a subclass with bar, I've tried to type an application like so:

/**
 * @constructor
 * @extends {Base}
 * @typedef {{
 *   bar: function(number)
 * }}
 */
let MixedBase = Mixin(Base);

Closure gives an error:

JSC_TYPE_PARSE_ERROR: Bad type annotation. type annotation incompatible with other annotations. See https://github.com/google/closure-compiler/wiki/Bad-Type-Annotation for more information. at line 33 character 3
 * @typedef {{
   ^

[MatrixFrog]

We can probably fix this and transpile in a way that is correct, but it will probably be very hard to typecheck that code, so I'm kind of hesitant to do it. But I haven't wrapped my head around this fully yet; will need to dig into it more.

[ChadKillingsworth]

My thought was to generate every permutation of class from the names:

const Mixin = (superclass) => class extends superclass {
  // ...
}

class Foo extends Mixin(Bar) {}

Transpiles:

class $jscomp$Mixin$Bar extends Bar {}
class Foo extends $jscomp$Mixin$Bar {}

All this really does is dynamically create prototype chains right? It's still a single chain and it's still members of the prototype.

[justinfagnani]

Yes, it just creates prototype chains.

I think this expansion is assuming a lot about the mixin function - that it only returns a class expression and does no other computation. In practice we have some utilities that will modify a mixin function so that it does things like de-duplicate multiple applications on a prototype chain (not reapply if it has already been applied), or cache and reuse the result of an application to a prototype chain. See DeDupe and Cache here: https://github.com/justinfagnani/mixwith.js

So I think it'd be better to type the mixin function itself somehow. To say that it takes a constructor of type S and returns a constructor of type M extends S...

Please my abuse of the type annotations below, and in general the level of crazy, but this is close to what I would want to type check mixins:

import {Mixin} from '../mixwith/mixwith.js';

/**
 * @template {HTMLElement} S
 * @param {function(new:S)} superclass
 * @return {M extends S}
 */
const AwesomeElement =  Mixin((superclass) => /** @typedef M */ class M extends superclass {
});

class MyElement extends AwesomeElement(HTMLElement) {
}

A few things that I knowingly fudge:

1. How do I specify that a template extends a type? @template {HTMLElement} S is my attempt to do Java's <S extends HTMLElement>.
2. function(new:S) is my attempt to specify a first-class type, the metaclass of S
3. M extends S is a type operator that represents the type created by class M extends S{}
4. class M extends superclass: M doesn't exist in the outer scope, so it needs some way to be pushed up. /** @typedef M */ is that :/
5. I'm assuming that at the callsite for AwesomeElement(HTMLElement) the typechecker can calculate the result type and validate that AwesomeElement can extend HTMLElement (overrides are compatible).

[ChadKillingsworth]

Well that changes things. Generics may be the path forward:

/**
 * @template S
 * @param {function(new:S)} superclass
 * @return {function(new:M extends S)} this is the hard part ...
 */
const Mixin = (superclass) => class extends superclass {};

[justinfagnani]

Oh yeah, my @return {M extends S} is wrong, yours is correct.

[justinfagnani]

Just brainstorming about what a @mixin tag could look like. There are two parts to a subclass factory - the factory function, and the class expression/declaration that it returns. I think we would need to identify both parts, especially because a mixin function might not declare it's class locally, it could be in another function, or the mixin could be a composition of others:

/** 
  * @mixin // need to associate this mixin with the class expression that defines the type
  * @param {function(new:C)} superclass
  */
const MixinA = (superclass) => /** @mixinClass {MixinA} */ class extends superclass {};
const MixinB = ...;

/** 
  * @mixin
  * @extends {MixinA, MixinB} // this is just a composition, so it "extends" other mixins
  * @param {function(new:C)} superclass
  */
const MixinC = (superclass) => MixinA(MixinB(superclass));

[dimvar]

A couple quick notes.

1) We are not prioritizing new type-system features at the moment, while we're trying to migrate to the new type checker, and then add new features only there, not to both type checkers.

2) You can use structural interfaces (with @record) to type a class that extends some other class and uses a mixin, eg:

/** @record */
function ExtraProps() {}
/** @type {function(number)} */
ExtraProps.prototype.bar;

/**
 * @constructor
 * @extends {Base}
 * @implements {Bar}
 */
let MixedBase = Mixin(Base);

[rictic]

Is there a design doc or tracking bug or similar for the migration to the new type system?

[dimvar]

Nothing public. Sent you some info internally.

[justinfagnani]

@dimvar what does @record do in your example? I don't see how it connects to the mixin.

[dimvar]

It tells the compiler that the extra properties on MixedBase are coming from Bar. If I understood your example, that's what you were intending with the typedef, no?

TypeScript also had a post on how to do mixins using structural types:
https://www.typescriptlang.org/docs/handbook/mixins.html

[justinfagnani]

I don't see any connection between ExtraProps and Mixin. Should ExtraProps be named Bar?

[dimvar]

Oh yes, sorry for the typo.

[ChadKillingsworth]

The following snippet proves the pattern can work, but it has a drawback. I haven't found a way to tell the compiler that Sub inherits it's implementation of the MixinInterface from BaseWithMixin. Right now, you have to add a stub definition: /** @type {function(number)} */ Sub.prototype.b;

/** @record */
function MixinInterface () {}
/** @param {number} n */
MixinInterface.prototype.a = function(n) {};
/** @param {number} n */
MixinInterface.prototype.b = function(n) {};

/** @constructor */
function Base() {
  console.log('Base.constructor');
}

/** @param {number} n */
Base.prototype.a = function(n) {
  console.log(`Base.a(${n})`);
};

/**
 * @template T
 * @param {function(new:T)} superclass
 * @return {?}
 */
function Mixin(superclass) {
  /** @constructor */
  function mixed() {
    superclass.call(this);
    console.log('Mixin.constructor');
  }
  mixed.prototype = Object.create(superclass.prototype);

  /** @param {number} n */
  mixed.prototype.a = function(n) {
    superclass.prototype.a.call(this, n);
    console.log(`Mixin.a(${n})`);
  };

  /** @param {number} n */
  mixed.prototype.b = function(n) {
    console.log(`Mixin.b(${n})`);
  };

  return mixed;
}

/**
 * @constructor
 * @extends {Base}
 * @implements {MixinInterface}
 */
var BaseWithMixin = Mixin(Base);

/**
 * @constructor
 * @extends {BaseWithMixin}
 * @implements {MixinInterface}
 */
function Sub() {
  BaseWithMixin.call(this);
  console.log('Sub.constructor');
}
Sub.prototype = Object.create(BaseWithMixin.prototype);

/** @type {function(number)} */
Sub.prototype.b;

/** @param {number} n */
Sub.prototype.a = function(n) {
  BaseWithMixin.prototype.a.call(this, n);
  console.log(`Mixin.a(${n})`);
};

const sub = new Sub();
sub.a(1);
sub.b(2);
sub.b('foo');

[ChadKillingsworth]

The following gets really close with NTI - it just doesn't properly understand the @extends part:

/**
 * @template T
 * @param {function(new:T)} Superclass
 */
function AddFoobar(Superclass) {
  /**
   * @constructor
   * @extends {Superclass} gives a warning about extending a non-object type
   */
  function Clazz() {
    Superclass.call(Clazz);
  }
  Clazz.prototype = Object.create(Superclass.prototype);

  /**
   * @param {string} input
   * @return {string}
   */
  Clazz.prototype.foobar = function(input) { return "foobar " + input; };

  return Clazz;
}

var Bar = AddFoobar(HTMLElement);


var bar = new Bar();

console.log(bar.foobar("foo"));
console.log(bar.foobar(1)); // This gives the expected type error
console.log(bar.tagName); // This gives a property never defined error

[ChadKillingsworth]

Use case description: http://justinfagnani.com/2015/12/21/real-mixins-with-javascript-classes/

[alexeagle]

We also want to use this for Angular, in our workaround for inheriting from builtins like Error.
/cc @rkirov

[ChadKillingsworth]

@alexeagle There is a Google doc with the design for this fix. @dimvar Can send it to you internally.

[dimvar]

@alexeagle I submitted a CL recently to support this in the new type inference. See 1fdf8fd4b3c3d8efd694b7e2bb29010fad49d7c2. We don't plan to support it in the old type checker.