[Glimmer2] Implement get helper

[ro0gr]

Left to do:

• Figure out how to setup {{input}} helper
• Port tests using input and mut
• Figure our what is the purpose of get() for the GetHelperReference
• tests for get()
• handle null for sourceReference and propertyPathReference (marked tests as [htmlbars] only)

[ro0gr]

Can't find when get() is supposed to be called. It's not invoked with tests.
I believe I'll find the answer but any guidance appreciated.

[chadhietala]

I believe get is used for something that is opaque from the consumer.

{{!-- application.hbs --}}
{{foo-bar as |value|}}
  {{value.foo}}
{{/foo-bar}}

{{!-- templates/components/foo-bar.hbs --}}
{{yield (get someObj somePathThatPointsToAnotherObj)}}

[chadhietala]

Doesn't seem to be a test for this. I would suggest adding a test that does this type of yielding.

[chancancode]

In general, a Reference is an abstract data type with the following interface:

interface Reference<T> {
  value(): T;
}

A reference represents a "computation" (similar to a "stream"), and calling .value() will return the current value of that computation. For example, a reference representing {{foo}} will return the current value of the foo property in the template's context. This gives you a stable value/wrapper (hence the name "reference") to hold on to that you can query in the future to re-render the template with the latest value as needed.

A PathReference is an extension to the system:

interface PathReference<T> extends Reference<T> {
  get(key: string): PathReference<any>;
}

Implementing the PathReference interface means that the value()s of the computation supports a conceptual get operation. For the purpose of understanding this, you can apply the Ember.get semantics here – i.e. the value()s returned by this computation makes sense as input to the first argument of Ember.get.

Alternatively, you can also think about the get semantics in terms of the templates. Anything foo that can be used in a {{foo.bar}} position must implement PathReference. In this case, {{foo.bar}} will take the {{foo}} reference as input and call .get("bar") on it to produce another stable reference (another PathReference, actually) representing {{foo.bar}}.

In practice, almost anything that can be used in a template needs to implement PathReference, because in handlebars almost anything can be used in that position. In addition to @chadhietala's example, there are a few other ways that a get helper can get into that position: {{(get foo bar).baz}}, {{#with (get foo bar) as |gotBar|}} {{gotBar.baz}} {{/with}}, etc.

In this case, the reference you are implementing is representing the result of the (get foo bar) computation over time, where foo and bar themselves are also PathReferences. The get(path) method (confusingly named in this context) should produce a reference representing the (get foo bar).path computation.

Putting everything together, I think something like this would work:

...

toReference(args) {
  let sourceReference = args.positional.at(0); // foo in (get foo bar)
  let propertyPathReference = args.positional.at(1); // bar in (get foo bar)

  if (isConst(propertyPathReference)) {
    // (get foo "bar"), so `propertyPathReference` is a reference whose value cannot change ever
    // this is pretty pointless, you could have just typed `{{foo.bar}}`
    return sourceReference.get(propertyPathReference.value());
  } else {
    return GetHelperReference(sourceReference, propertyPathReference);
  }
}

...

class GetHelperReference {
  // ...mostly following your implementation

  get(path) {
    new PropertyReference(this, path);
  }
}

[wycats]

Another thing to understand is why you need PathReference in the first place. Certainly, you can just call .value() on the parent and use a regular property lookup on the reified value.

The reason for PathReference is that the underlying root reference might have additional information about when you need to reify the value for paths derived off of it:

1. in Ember, if nobody called .set() on the parent object, you don't need to recompute the value.
2. in Ember, if you're looking at a computed property, you can rely on the computed property cache.
3. when working with immutable data structures, you only need to recompute the value if the root of the data structure has been replaced.
4. when working with primitive values, the get() virtually always produces a null value.
5. when working with non-configurable, non-writable values, you only need to compute the value once.

In other words, there's all kinds of information in the JavaScript data layer about how to most-efficiently derive a path off of a value, and PathReference allows the owner of the data to encode that strategy.

In Ember, this results in significant efficiency gains when Ember objects are used. Those gains would be portable to other data management strategies like immutable.js without forcing every data structure in your entire app to use Ember objects or immutable.js.

[ro0gr]

Thank you guys for such comprehensive explanations. It's definitely more than I could expect!

[ro0gr]

Sorry for a long progress. Some irrelevant tasks take much time.

I've added get implementation in GetHelperReference as @chancancode proposed and an appropriate yield test according to @chadhietala request.

Currently when you try to

this.sourceReference.get(this.pathReference.value()).value();

when the sourceReference has a falsy underlying value it produces the following error:

Cannot call get with 'apple' on an undefined object.

That's why null check tests are marked as htmlbars only for now.

[ro0gr]

Any suggestions for a better name here? GetStream looks like an htmlbars term

[chancancode]

👍 I agree. I think the GetStream is just referring to the nested get helper. Maybe just "should be able to use the result of a get helper as the key"?

[chancancode]

@ro0gr I probably need to circle back to this after EmberConf (at least after my talk 😄). Meanwhile, I wrote up the stuff we talked about in more details here: glimmerjs/glimmer-vm@master...chancancode:docs

[ro0gr]

@chancancode no problem. I'm not in hurry.
Thank you for the link.

[chancancode]

nit: we usually have a new blank line here for the new tests

[chancancode]

should probably do the I-N-U-R steps here to test that it can go from null to something else (and back)

[chancancode]

@ro0gr sorry for the delay! I left some comments. Looks good to me otherwise!

[chancancode]

Ah, one more thing – can you rebase and squash?

[ro0gr]

Thank you @wycats for merging this work and sorry again for such a long progress on finalizing this.

Anyway I've walked through your comments @chancancode. There were so much small issues. Hope I've fixed all of them in the latest commit.

[krisselden]

Sorry we dropped the ball linking that this PR was superseded by #13264 which addressed the above comments and was merged yesterday.

[ro0gr]

@krisselden I've seen this @wycats pr.
Actually there are some unaddressed issues wich are fixed here:

• add bunch of missing I-N-U-R steps according to @chancancode comments
• add missing @htmlbars should handle object keys as nulls

[ro0gr]

Should it be delivered via a new pr or left as it is?

[krisselden]

@ro0gr thank you

[ro0gr]

@krisselden and thank you 😄