Class-based approach for typed object model

[dfreeman]

I started to take a pass at introducing generics for enumerables like Ember.ArrayProxy based on the changes in #1 , but found that the approach there prohibited usage like:

class MyCustomProxy<T> extends Ember.ArrayProxy<T> {
  // ...
}

The reason for that is that there's no notion of a polymorphic constant in TypeScript, i.e. there's no way to do something like const ArrayProxy<T>: EmberClass<ArrayProxy<T>>.

The main notable change is the removal of the EmberClass<T> interface in favor of declaring actual ES6 classes. I'm also taking advantage of the class/interface merging trick mentioned in microsoft/TypeScript#340 (comment) to avoid needing to repeat interface methods in each class definition.

@dwickern if you have some time to play with this at some point, I believe I've captured the same safety guarantees as you have with the EmberClass<T> approach, but I haven't worked with TS's type system extensively, so it's entirely possible I've missed something :)

[dwickern]

Great find, this is really interesting!

I'd love to get rid of EmberClass<T> and have a less awkward API to extend Ember classes. If I understand correctly, someone defining an ember subclass would previously use:

interface MyClass extends Ember.Object { ... }
const MyClass: EmberClass<MyClass>;

And will now use this instead:

// if no interfaces/mixins are needed:
class MyClass extends Ember.Object { ... }

// if interfaces/mixins are needed:
interface MyClass extends SomeInterfaceOrMixin { ... }
class MyClass extends Ember.Object { ... }

That looks much better!

[dwickern]

One side effect I can see is that arguments to method overrides are no longer inferred. For instance since we know the argument types to setupController we shouldn't have to specify them:

// working in EmberClass<T> approach, not working in ES6 class approach
const MyRoute1 = Ember.Route.extend({
    setupController(controller, model) {
        controller.set('model', model);
    }
});

// not working in either approach
class MyRoute2 extends Ember.Route {
    setupController(controller, model) {
        controller.set('model', model);
    }
}

After reading microsoft/TypeScript#6118 and the linked issues, typescript surprisingly doesn't support this! So rather than implementing it in our bespoke object model, I'd rather remove it and wait for an official solution

[dfreeman]

Ah, I totally missed that that was happening before! (And didn't stop to reflect on why I'd unthinkingly made that change in this file). While it's pretty cool that that was working, I'd agree that sticking as close to the vanilla object model as possible will hopefully yield us the best results in the long run :)

[dfreeman]

// if interfaces/mixins are needed:
interface MyClass extends SomeInterfaceOrMixin { ... }
class MyClass extends Ember.Object { ... }

Thinking a bit more, rather than relying on merging we could use the same pattern in the definitions as an actual implementation would:

class MyClass extends Ember.Object.extend(SomeMixin) { ... }

I think we'd need to reverse the order of the clauses in type Extend = in order to get the inference to prefer the (this: ..., mixin1: Mixin<...>, args: undefined) signature over treating the mixin as the value for args, but otherwise that should produce the correct type.

My guess is that this form factor would be more immediately apparent to a reader as to what's happening, and it's a plus to have the declaration code mirror what an actual implementation would look like.

[dwickern]

class MyClass extends Ember.Object.extend(SomeMixin) { ... }

Yeah, that's a bug in both our branches