[@types/ember enhancement] - Cleanup around Ember.Object.{extend, reopen}

[mike-north]

Which package(s) does this enhancement pertain to?

• @types/ember

Please write a user story for this feature

As a maintainer of the ember type information, I want to remove some hacks around Ember.Object.extend that were necessary in TS 2.x so that the types are more approachable to existing and future contributors.

---

related conversation: https://discordapp.com/channels/480462759797063690/484421406659182603/489476577529167875
related code change:
mike-north/DefinitelyTyped@690bfed

The main part of this cleanup effort aims to do two things

1. Ensure that Ember.Object.extend and Ember.Object.reopen return a type that accurately represents what these functions actually do.

Conceptually, these functions essentially return the union of their input

class A { foo: 'hello' }
const B = A.extend({
   foo: 65
});
let b = new B();
b; // string | number

In the above example, we're lying about the type of b. It will definitely be a number because we override the original property when defining B.

In terms of implementation the way to do this is to define a new utility type that's the union of B and anything on A that is not present on B

B ∪ (A - B)

The TS implementation looks like

type Mix<A, B> = B & Pick<A, Exclude<keyof A, keyof B>>;

Then we can go and update types that currently look like this

static extend<...>(
    this: Statics & EmberClassConstructor<Instance>,
    arg1: MixinOrLiteral<T1, B1> & ThisType<Instance & T1>,
    arg2: MixinOrLiteral<T2, B2> & ThisType<Instance & T1 & T2>
): Objectify<Statics> & EmberClassConstructor<Instance & T1 & T2>;

to using Mix<A,B> instead of A & B

static extend<...>(
    this: Statics & EmberClassConstructor<Instance>,
    arg1: MixinOrLiteral<T1, B1> & ThisType<Instance & T1>,
    arg2: MixinOrLiteral<T2, B2> & ThisType<Instance & Mix<T1, T2>>
): Objectify<Statics> & EmberClassConstructor<Instance & Mix<T1, T2>>;

There are many reasonable test cases that currently fail, and this change would allow them to pass, and provide us with more maintainable types, and more accurate and complete type safety across some of the most commonly used areas of Ember (i.e., get() and set())

2. Remove a workaround that was necessary in the TS2.x release series to properly handle things like _super and statics through an ineritance chain in a typed ember addon

related issue: typed-ember/ember-typings#29)

There's a difficult-to-test (in the way we typically test ambient types) but reproducable issue involving extending from types through an addon in pre-3.0 versions of TypeScript.

Example:

My app uses ember-resize, which provides its own types that refer to @types/ember.

You can then try things like

import ResizeService from 'ember-resize/services/resize';

const Another = ResizeService.extend({});
class Another2 extends ResizeService.extend({}) {}
class Another3 extends ResizeService {}

const a1 = Another.create();
const a2 = new Another();
const a3 = Another2.create();
const a4 = new Another2();
const a5 = Another3.create();
const a6 = new Another3();

and you'll get an error on each of the a1 - a3 declarations. The message will have something to do with static properties like prototype. Tracking down the root cause for this reveals that the conflict/collsion is around the static properties of each class in the inheritance chain.

The workaround @dwickern and @dfreeman figured out was to make the statics Readonly<T> in .extend's return type.

In TS 3 this is no longer necesary, and the types become more flexible by removing this workaround.

[mike-north]

In accordance with our TS version support policy we can only drop support for TS 2.9 when TS 3.1 is released (maintaining our range of current - 1 releases).

It's likely that we'll be able to keep TS 3.0 or 3.1 as the minimum support version for a long time, providing a fix or workaround for this can be quickly found and released

[dwickern]

class A { foo: 'hello' }
const B = A.extend({
   foo: 65
});
let b = new B();
b; // string | number

Should we allow this? The equivalent code in regular typescript doesn't compile.

class A { foo: 'hello' }
class B extends A {
    foo: 65 // Property 'foo' in type 'B' is not assignable to the same property in base type 'A'.
}

You could use a type annotation if it was really your intent.

class A { foo: string | number = 'hello' }

[mike-north]

Should we allow this? The equivalent code in regular typescript doesn't compile.

No, but that's the current behavior. it should be

b; // number

---

EDIT: Rereading this, you could have meant "should we even allow this situation to type-check"

I think here we have to look to the code that the types describe. In this case we have to determine whether

class A extends EmberObject { foo: 'hello' }
const B = A.extend({
   foo: 65
});

is valid and supported use of the .extend function. If it is, we need to support it, and reflect the type of the result accurately.

[chriskrycho]

This sat for a loooong time, and since as part of Ember RFC 0800 we decided to drop support for complex type inference in .extend() in favor of telling people to use native classes (which actually work), I'm closing this. It's been a long time coming!