Implement {{get}} Keyword

[jmurphyau]

First rough shot at implementing the {{get}} keyword. Originally a helper it now needs to be a keyword to function correctly.

This work has come off the back of #11051 as well as a few comments from @wycats and @mmun about providing ember-get-helper directly in Ember.

[mmun]

You shouldn't need to manually call subscribe in your implementation. You should use replace inside of the compute hook so that the stream revalidation work is done as lazily as possible. More details below.

[jmurphyau]

The main thing I think I require is calling notify when the key stream is updated (if it's a stream). Because the sourceDep is neither of the two passed in streams (obj or key) I couldn't see how an update to the key value could trigger a notify without doing this.

[mmun]

You can compare your implementation with mine (I called it DynamicPathStream instead):

https://gist.github.com/mmun/219ddf4c33ce33e6ad76

The path stream is implicitly subscribed to on line 18. (It's not immediately subscribed to, dependency only set up their subscriptions lazily).

The object stream is indirectly subscribed to via line 39. In a nut shell, the valueDep depends on the object's stream key streams which in turn depend on the object stream itself.

[mmun]

The blankValue thing seems kinda hacky. Is that a bug we can fix?

[mmun]

Also, for the inline case you could probably invoke env.hooks.inline instead of duplicating the logic. As an example you can refer to the unbound kewyord:

https://github.com/emberjs/ember.js/blob/master/packages/ember-htmlbars/lib/keywords/unbound.js#L25

[jmurphyau]

The blankValue and the comment inside it is very tacky - yes. I just made a note/dedicated function for the purpose of pointing it out..

This line in htmlbars is where the issue is.. If a keyword previously returned a truth like value then returns a false like value - this line stops the update to the false like value and the old value is still displayed on the page. I'll try dig up a bit more detail and log it against htmlbars.

[jmurphyau]

I believe it occurs in the inline if helper since I'm using that in my tests. {{if true (get obj value)}} - when (get obj value) returns a false like value the if helper did not update with the new value.

[jmurphyau]

I found with the inline hook I either got into a infinite loop or got an error saying helper could not be found - depending on the value of path..

I originally used attribute to not replicate this logic but the documentation around it implied it shouldn't be used for the way I was using it.. I couldn't find anything more appropriate.

[mmun]

Ok so basically you should use the keyword to munge the params (e.g. build a GetStream out of params[0] and [1].), but then redirect it to a pure helper. Roughly should look like this:

registerKeyword('get', getKeyword);
registerHelper('-get', getHelper);

---

function getHelper([value]) {
  return value;
}

---

function getKeyword(morph, env, scope, params, hash, template, inverse, visitor) {
  var getStream = new GetStream(params[0], params[1]);

  if (morph === null) {
    return getStream;
  } else {
    env.hooks.inline(morph, env, scope, '-get', [getStream], hash, visitor)
  }

  return true;
}

Internally, we're going to make this pattern (munge the params in the keyword and delegate to a pure helper) simpler to use, but for now it is a bit boilerplate-y. In the future we'll probably just need to write

export default {
  link(env, scope, params, hash) {
    params[0] = new GetStream(params[0], params[1]);
    params.length = 1;
  }
};

[mmun]

Thanks a lot for working on this :)

[jmurphyau]

No worries (:

[jmurphyau]

@mmun I found I couldn't get my tests to pass without adding the obj as a dependency. It has to do with subexpressions because the only part of the tests that fail are the subexpression parts.

I can't seem to work this one out - do I even have a problem there?

[mmun]

I am pretty sure this can be removed with some more digging, but it's not a big deal to have extra subscriptions. I can take a closer look after it's merged.

[mmun]

It's pretty much good to merge. Last thing: You need to feature flag this because it's a new API.

[jmurphyau]

Ok awesome. I've just finished it all off based on the feedback/discussion so far. Will add feature flag as the next step.

[mmun]

Sweet. Please squash your commits and I'll merge.

[rwjblue]

👍

[jmurphyau]

@mmun thanks for your help with this PR - I appreciate it. Learnt a lot more about HTMLBars in the process.

[mmun]

❤️ ❤️