ES5 Getters

[chancancode]

Rendered

[mikkopaderes]

If this is only for computed properties, I believe it adds another confusion since we need to use this.get() for normal properties.

[chancancode]

@rmmmp you won't have to use this.get() for either in the world proposed by this RFC. (In fact you never really "have to" use this.get() on "normal" properties.)

[rtablada]

Since this is an RFC for the underlying Ember.Object this should also remove the need for get with ember data and when resolving services correct @chancancode?

[kategengler]

A smattering of addons, some of which are popular, use unknownProperty https://emberobserver.com/code-search?codeQuery=unknownProperty

I assume they would either have to change the way they work, or get would still be required when using those addons?

[chancancode]

when resolving services

@rtablada if you mean

let obj = Object.extend({
  foo: inject.service()
}).create();

...then yes, in this case you should be able to do obj.foo

I'll let the Ember Data folks chime in for the other part of your question. I think most of their public API would not require .get. (@wycats also has a pending response to @kategengler's question which would be relevant.)

[buschtoens]

Just as a general note regarding @kategengler's #281 (comment): In the distant (?) future, we'd have Proxies.

[buschtoens]

For the curious: Babel 7 will support optional chaining.

[wycats]

A smattering of addons, some of which are popular, use unknownProperty https://emberobserver.com/code-search?codeQuery=unknownProperty

I assume they would either have to change the way they work, or get would still be required when using those addons?

The way to think about this is:

In today's world, every object that inherits from Ember.Object has an interface that requires consumers to use .get to access properties. This means that "you must use .get() to access documented properties" is implicit in the documentation for any library that uses Ember.Object.

After this change, you would still be able to use .get() to access properties on any subclass of Ember.Object, but it would no longer be the case that it was required.

In practice, what this means is that the handful of libraries that want to use unknownProperty need to start explicitly documenting .get() as part of their interface for getting properties from their objects.

For now, it would be reasonable for users to use .get() when interacting with third party libraries, until libraries have generally become more explicit about that. I would also encourage the community to help improve the documentation on this front.

[wycats]

Several additional points:

1. We internally use Ember.get() for interop with normal JavaScript objects, so this change shouldn't affect any part of interacting with Ember itself.
2. It's also not uncommon for JavaScript libraries to require .get() for their objects, so it shouldn't be too problematic if libraries that use unknownProperties are documented in this way.

[grapho]

Is there a separate RFC that will be introduced which will replace:
this.set('foo', 'bar')
with
this.foo = bar
?

I dont like being a negative noodle, but it seems to me that as long as set() exists.. get() should also exist for consistency sake.

That being said. I AM super excited to one day be able to set/get things with classic JS "dot style"

[chancancode]

@grapho does the "How We Teach This", "Drawbacks" and "Alternatives" section not answer your question?

Besides the empirical evidence that other frameworks have the same API, you can also construct a principled explanation/mental model for this: reading data is always side-effect free, but mutations are by definition effectful. Therefore, it is a feature not a bug to explicitly call out these mutations, both for human understanding (something something "mutations are the root of all evil" ¯_(ツ)_/¯) and for Ember (so it knows to re-render your code).

[chancancode]

We internally use Ember.get() for interop with normal JavaScript objects, so this change shouldn't affect any part of interacting with Ember itself.

Another way to put this is that if the addon is using unknownProperty (or proxies) for UI objects intended for templates (e.g. yielding it), then it shouldn't be affected by this – when you do {{foo.bar.baz}} in the template, Ember will do the equivalent of Ember.get(this, 'foo.bar.baz') which will allow proxies to work.

[grapho]

@chancancode good points!
I am in favor of a soft deprecation of get... just not generate deprecation warnings until actual full removal of get becomes closer to realization. Since gets are typically all over the place :p

[chancancode]

It's also not uncommon for JavaScript libraries to require .get() for their objects, so it shouldn't be too problematic if libraries that use unknownProperties are documented in this way.

A quick search turned up a few examples:

• http://docs.parseplatform.org/js/guide/
• https://developers.google.com/analytics/devguides/collection/analyticsjs/accessing-trackers

Then there are many other JS APIs that use getters (as in Java-style obj.getFoo()):

• https://developers.google.com/maps/documentation/javascript/3.exp/reference#Map
• https://www.mapbox.com/mapbox.js/api/v3.1.1/l-marker/#api

The point is, when interacting with third-party libraries (which an addon is) in JavaScript, sometimes you work with "plain properties", sometimes you are going through something like obj.get(), sometimes you are going through getter methods (obj.getFoo()) and sometimes you are going through some even more exotic APIs (obj.ref('foo').value()...).

So, as long as those special objects (those that need to use unknownProperties, usually proxy or proxy-like objects) are documented clearly, the mental model will be that get() is just another API on those the object you have to learn.

As @buschtoens said, in the very long term, the community (not just the Ember community, but the wider JS community) will transition to use the native proxies and expose a more uniform API. Until then, in the rare cases where you need to implement proxies in user-land (which, like ES5 getters, are not shimmable), those objects would have to go through something like .get() as the user-land trap, which is understandable IMO.

[Gaurav0]

The RFC does not make clear how we will know when we will be able to use this. Is it available now? After all, we can access some properties now without get.

[pzuraq]

@Gaurav0 I'm pretty sure this means for all computed properties specifically, which cannot be accessed without get currently

[chancancode]

@Gaurav0 currently the mental model I would teach is "always use .get". after the feature lands, you will not need it in general, unless the library/addon tell you otherwise.

As for timing, as always, it will be available when (and if) the RFC is merged, the feature is Go'ed by the core team and when it makes it all the way to the release channel. Might be available earlier on canary behind a flag for experimentation.

[e00dan]

I would love to see this implemented... ❤️

[sandstrom]

How far out are proxies?

If they would allow dropping set too, and keep support for unknownProperty perhaps it's better to wait a little longer — taking down both get and set in one swoop.

[buschtoens]

@sandstrom I think you pretty much answered your question yourself. 😛

IE's the only browser of interest that is dragging behind. Since it will never get Proxy support, we'd basically have to wait until Ember.js drops IE support completely.

[Panman82]

Can Ember utilize proxy-polyfill or does the polyfill not support enough of it to be worthwhile? I'm thinking ember-cli could include this based on targets.

[Edit] Perhaps this is the limiting factor.?.

The polyfill supports just a limited number of proxy 'traps'. It also works by calling seal on the object passed to Proxy. This means that the properties you want to proxy must be known at creation time.

[oriSomething]

I'm not sure it should be said here. But we can improve ergonomics of tooling such as TypeScript today with Ember.get. And if we will able to pass an Array to Ember.get like Ember.get(["some", "nested", "key"]) we would even have better ergonomics with TS

[chancancode]

I have tried in the past, and I am pretty convinced you cannot type an Ember object successfully today, even without considering nesting. You need to convince TypeScript that they are no properties on the object itself (so obj.foo etc are type errors) but when you do obj.get(“foo”) or get(obj, “foo”) it should have the right type. I have come close with using some phantom associated type tricks but I think it’s fundamentally impossible.

[chriskrycho]

For what it’s worth, you can, and we are... as long as you don’t do nested properties. It took a fairly ridiculous amount of work, though.

[chriskrycho]

To be precise: the types you end up with on the object itself end up having a type of ComputedProperty<TheType>, which the type system then properly unwraps when using get.

[bryanaka]

I am stoked about this RFC. When teaching people coming from other libs/frameworks, get usually comes with more questions than set because react's setState somewhat normalized the setting function

[chancancode]

This RFC was nominated and approved for Final Comment Period (FCP) yesterday, since most comments have already been replied to. The FCP with last for a week. If there are any substantial new arguments that are brought up during this time, the FCP will be restarted (or aborted). If you haven't reviewed the proposal yet, now would be a great time to do so.

If I may, here is a summary of the main objections:

1. It is confusing that set is required, but get is not.

   As mentioned in the RFC and pointed out by other commenters, React (and other libraries, and our experience with Glimmer.js) has proved that this is not much of a confusion point. At the same time, there is also confusion that comes with requiring get() in the first place. Since this is such a big improvement in ergonomics, we also don't think it is justifiable to hold this back the benefits until set is also eliminated, as it is not even clear whether it could be done or not.

2. It does not work with proxies.

   When used in templates (which is true for most use cases for proxies), this is not an issues as Ember/Glimmer internally still uses .get. However, we can explore adding development mode assertions (similar to the "mandatory setters" assertions) when we detect that this is a problem.

Meanwhile, I am working on implementing this feature behind a flag (which can be done independently from progressing this RFC). Once the patch is merged, I'll comment on this thread so you can try the feature on Ember Twiddle, or with the canary build.

[rwjblue]

It is currently possible to get a ComputedProperty object using this.fullName today.

It definitely is not intended (or public API), and ComputedProperty instances used in this way have a number of completely broken scenarios. The likely path forward for that scenario is that we will detect when it is done, and assert in development builds (likely via native proxies detecting .get being called and/or isDescriptor being accessed).

[chancancode]

@mike-north as mentioned in the RFC:

One caveat is that the computed property function is currently stored on the
instances for implementation reasons that are no longer relevant. However,
it is possible that some developers have observed their existance and have
accidentally relied on these private semantics (e.g. chancancode.fullName.get()
or chancancode.fullName.isDescriptor).

Before landing this change, we should turn the property into an assertion
for so that in these unlikely scenarios the developers will at least receive
some warning.

I have started working on that in #15992

[jelhan]

Sounds great. But one question isn't cleared out for me yet. Perhaps it's obvious but I'm not sure about.

How would that effect the need of consuming a computed property which should be observed? Would .get() still be required therefore? If so it should be made explicit in the How We Teach That section and maybe added as another example in Motivation section for use cases still requiring .get().

[buschtoens]

@jelhan It is not explicitly stated in the RFC, but the way I understand it, ES5 getters would basically trigger the exact same code the current obj.get(keyName) (property_get) triggers today. This means nothing would change.

You still have to pseudo-consume unconsumed observed properties in the init hook, for example:

init() {
  this._super(...arguments);

  this.myProperty;
}

[theseyi]

@buschtoens to clarify, did you mean ES5 getters? I think that's what the RFC is referring to

[buschtoens]

@theseyi absoultely. I updated my comment, thank you.

[chancancode]

Hello! I implemented this feature (behind a flag). You can try this on canary now: https://ember-twiddle.com/7aded3d76fcc04867b426bf05f641388?openFiles=controllers.application.js%2C

[chancancode]

Thanks everyone for your participation! Since no major issues were found during the FCP, we will now close and merge this proposal.

[wycats]

For what it's worth, I think it's important that we consider the developer experience of migrating to this flag an open issue that should be addressed during implementation.

The most notable version of the problem is existing code using Ember proxies, and what kind of development mode experience we could use to help people who forget to use .get() on those objects.

[chriskrycho]

Implementation question related to my above comments on TypeScript—how does this interact with the use of class instance properties?

import Service from '@ember/service';
import { computed } from '@ember/object';

class Neato extends Service {
  bar = 'a string';
  foo = computed('bar', function() {
    return this.bar.length;
  });
}

Will the proposed (and already-initially-implemented) solution handle this correctly?

---

Edit: experimenting with the Twiddle linked above suggests no: since class property instances are equivalent to running this.property = ... at the head of the constructor, you can simulate it by doing this with Godfrey's example:

import Ember from 'ember';

const { computed } = Ember;

export default class extends Ember.Controller {
  constructor() {
    super();
    
    this.appName = 'Ember Twiddle';

    this.firstName = 'Godfrey';
    this.lastName = 'Chan';

    this.fullName = computed('firstName', 'lastName', function() {
      let { firstName, lastName } = this;
      return `${firstName} ${lastName}`;
    });

    this.preferFirstName = true,

    this.preferredName = computed('firstName', function() {
      return this.firstName;
    });

    this.displayName = computed('preferFirstName', 'preferredName', 'firstName', function() {
      return this.preferFirstName ? this.firstName : this.preferredName;
    });
  }
}

And when you drop that into the Twiddle, you'll see the the computed properties do not update as they should. 😢

[pzuraq]

Yeah, the issue there is that currently the getters get applied once when the class is first defined (in the proto phase). Since class fields are per-instance, and we don't have a hook to finalize fields, there is no way to transform them into native getters. It also likely wouldn't be preferable anyways, since it would then be creating an instance of the CP per instance of the class, which could have some pretty negative perf implications.

I think the path forward for native classes and computed properties is using decorators, they're the only method we really have to modify the prototype of a class at this point (using just class syntax).

[chriskrycho]

That is the conclusion I came to as I experimented bit last night. 👍🏼 Come on, TC39, give us that decorator goodness!

[wycats]

@chriskrycho since you're using TypeScript, why not just use decorators?

[chriskrycho]

@wycats It's a good suggestion. Unfortunately, the result of our experiments with the ember-decorators in TS has been that they don’t work because Babel and TS have different implementations; we'd need to completely reimplement the package. As a result, we've essentially just opted to wait on decorators until we get Stage 3 decorators and Babel's and TS's implementations converge.

[pzuraq]

@wycats the main issue with decorators as they stand in Typescript is with class fields. Currently, there is no way to get the initializer for a class field in TS, so some of our decorators can't be made to work with it without breaking changes (specifically thinking of @className and @attribute).

I think we can take another stab at TS compatibility once Babel finalizes the Stage 2 transforms, since we'll need to rewrite ember-decorators' internals a fair amount at that point. Ideally though we would wait for Stage 3 to avoid more churn/breaking changes.

[mike-north]

For those looking to test this out in their own apps, here's a regex that works for most cases of find/replace.

⚠️ Use at your own risk ⚠️

REPLACE
\.get\((['"]{1})([\w\-\.]+)\1\)
WITH
\.$2

[rwjblue]

@mike-north - Hmm, I think it would be much better to use the codemod (which npx ember-cli-update --run-codemods would do for you automatically) mentioned in the 3.1 release blog post.

https://github.com/rondale-sc/es5-getter-ember-codemod

[chancancode]

Also (as mentioned in the RFC) – it's not generally safe to replace all usages of .get because your code or addons may return proxies, so some combination of caution, good test coverage and manual review is necessary here.

[mike-north]

@rwjblue I didn't know that existed, but have had bad luck with TS and jscodeshift

[rwjblue]

@rondale-sc and I recently wrote codemod-cli, to make authoring code mods much easier (the goal being to author only the codemod and it’s expected inputs and outputs and let codemod-cli do all the heavy lifting glue code). One of the out of the box features is typescript support. This allowed us to update ember-qunit-codemod to “just work” for typescript files. We should do the same for es5-getter-ember-codemod...