Add unused generics to ComputedProperty types for compatibility with @types/ember__object

[gitKrystan]

Ember's preview-types remove the generic Get and Set parameters from the ComputedProperty type that are included in @types/ember__object.

Unfortunately, this change caused compatibility issues for libraries that need to support both the @types packages and preview-types. While these libraries could drop support of "unwrapping" computed properties for get, this ends up being a pretty big change, as shown here: machty/ember-concurrency#512.

See machty/ember-concurrency#510 for more details. Until that issue is resolved, users of ember-concurrency are blocked from using preview-types.

Would love thoughts from @chriskrycho on this.

[dfreeman]

The original purpose of the ComputedProperty<T> type was to capture what you couldn't do with computed properties. Namely, given this code:

let MyObject = EmberObject.extend({
  foo: computed(() => 'hi'),
});

let obj = new MyObject();

Then at runtime, obj.get('foo') or get(obj, 'foo') would give a string, while bare obj.foo would give you a mostly-opaque object representing the computed property definition itself. The ComputedProperty type captured that distinction in the type system, forcing users to use get to get at the underlying type.

Since Ember 3.1, though, (thanks to RFC 281) computed properties have installed native getters on their host so that users don't have to use get any more. Accordingly, since 3.1 there isn't much of a reason to use ComputedProperty<Get, Set> any more—just about anywhere it appears can now just be whatever type Get is instead.

This change is the reason the ComputedProperty type, as well as that of get, were simplified in the native types—without the need for the wrapper, there also isn't really a need for the unwrapping.

In Ember 2.x, the TaskProperty vs Task distinction made sense, because I had to use get in a method like go() here:

let MyObject = EmberObject.extend({
  foo: task(function*() { /* ... */ }),
  go() {
    this.get('foo').perform();
  }
});

But in 3.1+, this.foo.perform() is equally valid, so rather than having TaskProperty extends ComputedProperty<Task<...>>, I think ember-concurrency should be able to directly do TaskProperty extends Task<...> instead. With that change, get will continue to work in older code that uses it, and writing this.task.perform() without the intermediary get will also (correctly) be allowed.

[gitKrystan]

@dfreeman I totally agree. To that end, I opened machty/ember-concurrency#512 as an alternative fix to this issue for ember-concurrency. It does break types for get though (I had to cast a bunch of unknown to the appropriate ember-concurrency type); are you saying there's a way around that?

[dfreeman]

It does break types for get though (I had to cast a bunch of unknown to the appropriate ember-concurrency type); are you saying there's a way around that?

Yep, I'm saying that the type of this.foo and get(this, 'foo') should be the same once you take ComputedProperty out of the equation, so if this.someTask.perform(...) works, then get(this, 'someTask').perform(...) should also work. That's what this signature should be accomplishing.

[kategengler]

Is this still relevant?

[gitKrystan]

I think it's no longer needed.