Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable multiple dynamic keys with {{get}} #12263

Closed
Panman82 opened this issue Sep 1, 2015 · 13 comments
Closed

Enable multiple dynamic keys with {{get}} #12263

Panman82 opened this issue Sep 1, 2015 · 13 comments

Comments

@Panman82
Copy link
Contributor

Panman82 commented Sep 1, 2015

Currently, I believe the {{get}} helper is limited to one dynamic key.

{{get someObject keyA}}

I have a case where there are multiple dynamic keys in a path. Eg: foo.bar.baz The current workaround is to use multiple, nested "get's".

{{get (get someObject keyA) keyB}}

What I propose is that the get helper use the remaining params as individual keys.

{{get someObject keyA keyB}} // == someObject.keyA.keyB

It almost acts like a concat helper, but that is not allowed in the current implementation.

{{get someObject (concat keyA '.' keyB)}}
@rwjblue
Copy link
Member

rwjblue commented Sep 1, 2015

Nested (get's seem OK/appropriate here to me. Why does that seem wrong to you?

@stefanpenner
Copy link
Member

@rwjblue im curious why the path variation is disallowed .

@Panman82
Copy link
Contributor Author

Panman82 commented Sep 1, 2015

@rwjblue It just seems like it would be a nice shorthand for deep key paths. And it just reads better, no?

There is just something about nested gets, even using the concat option, that makes by brain go "ick". And the multiple params just seems logical to me. Perhaps others feel differently.

@mmun
Copy link
Member

mmun commented Sep 1, 2015

@rwjblue I don't like this (get should act like Ember.get) but on the other hand, this is something that's easy to ast-transform if we have a better solution in the future.

@Panman82
Copy link
Contributor Author

Panman82 commented Sep 1, 2015

@mmun So this.get('foo.bar.baz'); is not the same??

@mmun
Copy link
Member

mmun commented Sep 1, 2015

The same as?

@btecu
Copy link
Contributor

btecu commented Sep 1, 2015

What do you use the {{get}} helper for?

@mmun
Copy link
Member

mmun commented Sep 1, 2015

@btecu The {{get}} helper is used when you need a dynamic member lookup, much like foo[bar] in JS. Examples:

  • {{get obj somePath}} is the template equivalent of Ember.get(obj, somePath).
  • {{get obj "foo.bar"}} is equivalent to {{obj.foo.bar}}.

From what I've heard it's typically used when dealing with polymorphic models. Often with dynamic forms.

@Panman82
Copy link
Contributor Author

Panman82 commented Sep 2, 2015

@mmun So, re-reading everything, sounds like you feel it should at least support {{get obj "foo.bar"}}, currently not allowed. That would then enable the use of {{get obj (concat dynamicFoo '.' dynamicBar)}}. But what do you mean about ast-transform?

@mmun
Copy link
Member

mmun commented Sep 2, 2015

@Panman8201 If that doesn't work I am sad. It should! Forget about the AST transform, I misunderstood the problem. We both agree that {{get obj (concat dynamicFoo '.' dynamicBar)}} should work.

@rwjblue
Copy link
Member

rwjblue commented Sep 2, 2015

@mmun - I believe @Panman8201 mentioned this in the intro description, but we currently disallow the key to include a period. See https://github.com/emberjs/ember.js/blob/v2.1.0-beta.2/packages/ember-htmlbars/lib/keywords/get.js#L123.

@Panman82
Copy link
Contributor Author

Panman82 commented Sep 2, 2015

Is it as simple as removing that assert? I don't see anything else holding back periods.

As for multiple dynamic keys, could that all be handled in the buildStream() function? Maybe by changing this line to something like:

const objRef  = params[0];
const pathRef = params.slice(1).join('.');

That would allow for either {{get obj foo bar}} or {{get obj (concat foo '.' bar)}}. I realize the multiple dynamic keys option isn't that popular with ya, but I'm just trying to figure out how that could be enabled.

@rwjblue
Copy link
Member

rwjblue commented Sep 10, 2015

Should be fully addressed by #12323.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants