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

Add support for computed property query param values #12

Open
SeanSmithers opened this issue Jul 23, 2015 · 7 comments
Open

Add support for computed property query param values #12

SeanSmithers opened this issue Jul 23, 2015 · 7 comments

Comments

@SeanSmithers
Copy link

e.g.:

<a href="{{href-to 'apps.app.inbox.search.conversations' app (query-params tag=tagOrSegment.id)}}">{{unbound tagOrSegment.name}}</a>

Results in: ...inbox/search/conversations?tag=%5Bobject%20Object%5D

@GavinJoyce
Copy link
Contributor

@SeanSmithers Can you still reproduce this? I tried to in the branch, but it seems to work fine:

#22

Perhaps you could modify my branch to include an example of it not working?

@SeanSmithers
Copy link
Author

@GavinJoyce No, I can't seem to reproduce it now, I pushed something to your branch to demonstrate what I was trying

@GavinJoyce
Copy link
Contributor

I think this may only be an issue in Ember 1.x. I can't reproduce it in ember 2.0. I'll confirm soon

@dongintercom
Copy link

@GavinJoyce @SeanSmithers

This behaviour (%5Bobject%20Object%5D) can be reproduced when the attribute is KeyStream object and its value is not immediately available (e.g like an Ember model). In this case, link-to needs to be used because it has a view and can refresh the view when the value changes (gets notified from subscription).

href-to is supposed to be a lightweight alternative, if I understand correctly, subscribing to the changes of some lazy attribute and re-render will not be supported since it would require a view, and href-to is a helper that can generate links in a straight-forward way.

So we maybe would need to state this corner case explicitly in the README, what do you think?

@GavinJoyce
Copy link
Contributor

@dongintercom thanks, that's useful. I'll use this to create a failing test and use that to fix the issue

@buschtoens
Copy link

[...] subscribing to the changes of some lazy attribute and re-render will not be supported since it would require a view [...]

Not necessarily, I believe.

Quoting from emberjs/ember.js#11021 (comment):

For anyone coming across this later, helpers in Ember 2.x are now "real" objects and can have access to services (via Ember.inject.service or initializer based injections).

Example:

export default Ember.Helper.extend({
 i18n: Ember.inject.service('i18n'),

 compute(params, hash) {
   let i18n = this.get('i18n');
   // stuff here
 }
});

I haven't actually tested this, but we should be able to wait for Promise resolution and call recompute.

However, we then would have to actually subclass Helper instead of using plain objects which could lead to some slight performance degradation. If my assumption checks out, maybe we could provide a second promise-aware helper?

@shelby-carter
Copy link

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

Successfully merging a pull request may close this issue.

5 participants