Discussion - ES5 getters for computed properties

[mike-north]

trafficstars

As it stands now (and because this was the best choice for pre-Ember-3.1 versions) this example will not type-check

import Component from '@ember/component';
import { computed } from '@ember/object';

export default class XFoo extends Component.extend({
  fullName: computed('firstName', 'lastName', function() {
    return `${this.firstName} ${this.lastName}`;
  })
}) {
  firstName = 'Tobias';
  lastName = 'Fünke';
  bar() {
    this.fullName.length; // 💥
    // [ts] Property 'length' does not exist on type 'ComputedProperty<string, string>'.
  }
}

Moving forward, the common practice will be for developers to avoid using get, and to instead directly access computed properties.

One idea would be to steer people toward overriding ComputedProperty with a "passthrough" type alias

// types/@ember/object/computed.d.ts

type ComputedProperty<Get, Set = Get> = Get;
declare const ComputedProperty: ComputedProperty<any>;

export default ComputedProperty;

Eventually, we may want to simplify (and likely speed up the types) by removing all of the CP mapping stuff if we can -- there are many places where ComputedProperty<string, string> can simply be represented as a string when it comes to Ember.get and Ember.set.

[mike-north]

It's also worth noting that if we determine it's possible (and desirable) to eliminate a large portion of the CP mapping stuff, doing that along with the Object.{extend,reopen} cleanup (#291) would result in a substantial simplification of the types. They'll likely run faster, be easier to maintain, etc...

AFAIK, this is the list of things that still need get()

• If you are calling get with a chained path. For example in this.get('a.b.c') if b is undefined the return value is undefined. Converting this to this.a.b.c when b is undefined would instead raise an exception.

We can't provide type safety for this situation anyway, so we lose nothing here

• If your object is using unknownProperty you must continue to use get. Using an ES5 getter on an object with unknownProperty will cause an assertion failure in development.

Ditto -- unknownProperty is a very infrequently used feature of Ember's object model, and for obvious reasons does not mix well with types. I don't see us losing much here

• Ember Data returns promise proxy objects when you read an async relationship and from other API. Ember proxy objects, including promise proxies, still require that you call get to read values

If this is all that's left, we can come up with a more localized solution around hasMany and belongsTo

[chriskrycho]

• On the first one: the chained path thing isn't so much "requires Ember.get" as "requires something as smart as it is about not to blowing up on undefined path entries." People could just as easily use _.get from lodash; it'll work just as well.

• On Ember Data and proxies: I'm perfectly fine with defining a new (non-exported!) type that is something like ProxiedValue<Get, Set = Get> that is basically identical with our current ComputedProperty type, but applied only to those specific contexts.

[chriskrycho]

Similar to #291, this has become irrelevant in light of Ember RFC 0800, the switch to supporting only native classes with official types, and the move away from classic CPs to only using native getters.