Add a type parameter to Enumerable and friends

[dfreeman]

This change makes Ember.Enumerable and its descendants polymorphic, allowing for better type safety in their properties and methods.

[dfreeman]

For whatever reason the tests were passing locally, but based on Travis and microsoft/TypeScript#17829 I may need to rethink this slightly.

[dwickern]

Can we do the class/interface merging trick?

interface ArrayProxy<T> extends MutableArray<T> {}
class ArrayProxy<T> extends Object.extend(MutableArray) {}

[dfreeman]

We can, but we'd need to do something like this to avoid a conflicting merge between MutableArray<T> and MutableArray<any>:

interface ArrayProxy<T> extends MutableArray<T> {}
class ArrayProxy<T> extends Object.extend(MutableArray as {}) {}

Since this is just a declaration file, we could actually get away without the .extend() call at all, but real code would need it to pick up the actual mixin implementation.

This is super ugly, but given that type variables apparently aren't in scope in base class expressions, we're pretty restricted. I'm not sure I have a better alternative to propose.

[dfreeman]

Doing a quick code search on EmberObserver, code using the enumerable family of mixins only appears in a handful of places, and it tends to be in fairly low-level "plumbing" classes.

Given that, it may not be the end of the world that getting the proper typing takes some gymnastics, but it sure seems unfortunate :/

[dwickern]

As long as generic mixins aren't a common thing, this should be OK

[dwickern]

This is a good start. Going forward I think we can strengthen the types a bit.

I noticed you have to overloads for mapBy:

mapBy<K extends keyof T>(key: K): GlobalArray<T[K]>;
mapBy(key: string): any[];

Maybe it's worthwhile to separate out these "unsafe" methods which deal with path strings. They could be opt-in like the prototype extensions. We could also offer a typescript-specific API on top of ember which solves this. (e.g. Array.mapByPath(k1, k2, ..., kn))

[dwickern]

We could work around the issue of extending generic mixins using ES6 classes

interface MutableArray<T> { ... }
class MutableArray<T> extends Mixin<MutableArray<T>> {
  private constructor();
}

That's slightly different from what we discussed before about getting rid of the Mixin type parameter entirely. I don't feel too strongly either way.

[dfreeman]

Maybe it's worthwhile to separate out these "unsafe" methods which deal with path strings. They could be opt-in like the prototype extensions. We could also offer a typescript-specific API on top of ember which solves this. (e.g. Array.mapByPath(k1, k2, ..., kn))

Yeah, my thought with the overload was to provide some inference where possible without totally breaking things for the nested path use case. I agree it would be interesting to try and consider API patterns needed across the board to improve typeability for operations on key paths — things like get(), dependent keys for computeds, etc.

[dfreeman]

We could work around the issue of extending generic mixins using ES6 classes

We could, but it only really solves the problem for declaration files—since actual Ember mixins don't work that way, there would always be a disconnect between the typings and the implementation. Like you I don't have a super strong opinion, but my leaning is probably to stick with the status quo until we start seeing more generic mixins pop up.

There's been some talk on the subject of mixins with ES classes in the #topic-es-decorators channel in Slack, but no strong conclusions yet. Decorators in TS unfortunately don't have an impact on the type of their target at this point, though, which is unfortunate.