-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
delegate is not a function
#11592
Comments
Definitely would be helpful. |
Managed to reproduce in JSBin, it looks to be if the query param is a computed property: |
@rlivsey what's your use case for using a computed property as a QP's default value? I honestly don't expect that to work, and if it did before, it might have been an accident, but I'd like to understand how you are using it in your app. |
My use-case is to display a formatted const DATE_FORMAT = 'YYYY-MM';
export default Ember.Controller.extend({
queryParams: {
dateStr: "date"
},
date: null,
dateStr: Ember.computed("date", {
get() {
const date = this.get("date");
return date ? date.format(DATE_FORMAT) : null;
},
set(k, v) {
this.set("date", window.moment.utc(v, DATE_FORMAT));
return v;
}
})
}); I suppose instead I could instead invert this behaviour and instead of dateStr: null,
date: Ember.computed("dateStr", {
get() {
const dateStr = this.get("dateStr");
return dateStr ? window.moment.utc(dateStr, DATE_FORMAT) : null;
},
set(_, date) {
this.set("dateStr", date ? date.format(DATE_FORMAT) : null);
return date;
}
}) This works, but it would be nice to have the option to do it the other way. If using a CP for the value, it could just assume the default is |
This has burned me on the 2.0.0 release as well. Had to revert back to 1.13.8 because I have a lot of this sort of thing going on. |
Thought I'd mention I just ran into this too. Similar story to @rlivsey... |
I ran into this again the other day, similar use-case where I want the query string to reflect the current selected person in a list. The most straightforward approach is to simply alias the query param to the selected person's ID: export default Ember.Controller.extend({
queryParams: {
selectedID: "selected"
},
selected: null,
selectedID: Ember.computed.reads("selected.id")
}); That doesn't work, so now I'm forced to make this more complex and should also leave a comment to make sure a colleague doesn't try and refactor it back to the simpler version: export default Ember.Controller.extend({
queryParams: {
selectedID: "selected"
},
selectedID: null,
// query params doesn't support computed properties, so set the ID manually
// see issue for more details: https://github.com/emberjs/ember.js/issues/11592
selected: Ember.computed({
set(_, person) {
this.set("selectedID", person ? person.get("id") : null),
return person;
}
})
}); |
Possibly related: #12102. |
I am also getting this error after upgrading from 1.13.8 to 2.0.2. I'm not even using query param aliases and they are normal properties. I noticed that it only occurs when I set the query params controller: queryParams: ['from', 'to', 'period', 'preset'],
// Default date range
preset: 'past 7 days',
period: 'day',
from: 0,
to: 0,
...
setDatesFromPreset: on('init', observer('preset',
function setDatesFromPreset() {
const preset = this.get('preset');
const {
from,
to,
period,
} = ReportController.datePresetQueryParams(preset);
if (!isNone(from) || !isNone(to)) {
this.setProperties({
period,
_from: from,
_to: to,
});
}
this.set('toggleLabel', Ember.String.capitalize(preset));
})
), |
So are query params in Ember 2.x effectively broken? 😕 PS What's Canary in subj's title is now Stable. |
having same issue |
Also have a problem (in all browsers) with query params and computed property with get\set specified. |
I just ran into this issue too. |
+1 |
Have been there: mainmatter/ember-simple-auth#743 |
+1 |
1 similar comment
+1 |
This came up in the Core Team discussion for 2015-10-23. Initial default values on query params were always intended to be statics but the language didn't give us the ability to express that. There weren't any tests around using a computed property here because nobody thought people would use them in this way. As @machty said:
Basically, using a computed property in this way only worked coincidentally in some cases and a change has removed those cases. However, using a computed property this way likely introducing insidious edge-casey bugs for some interactions between a query param and controller because the the value was being looked up on the controller's See here and here for the relevant code before the changes. We'd accept a PR for 1.x that fixes if it also included enough tests demonstrating that there aren't any weird interactions between query params / controller when the query param property is a computed property. We could include this in the next 1.13.x release for people migrating. We wouldn't carry this behavior over to 2.0 because it definitely won't be possible with the various upcoming router changes. @rlivsey's solution should allow people to avoid this #11592 (comment) |
Fair enough! Would be good to have a nice descriptive error message pointing people in the right direction & making it clear it's not a bug. I'll take a stab at this shortly. Any pointers / advice on how to detect a CP being used in this way would be great. |
For anyone interested, I've tried a basic example on JSBin and it seems to work fine: |
+1 |
Also experiencing this. To give a little more detail for my particular case, it seems that at the time The sequence of events I can so far infer is:
Note: I'm not using any computed properties on my Route. I am using ember-cli-pagination though. This bug occurs when setting the an initial static value. |
/cc @luisah |
So from Slack @rwjblue was suggesting writing a QPs service that basically allows you to subscribe to changes on a single QP in that service. Would love to flesh this out and maybe write an addon and RFC. |
+1 just experienced this upgrading to |
@knownasilya I've been using a similar QP setup (service + mixins) in my company's main app since 2.0, and I would love to use your addon instead and help with the RFC. |
@2468ben would love to have you on board. Feel free to submit issues/PRs. |
I just found this too in my app. How can I help? Reproducible example? |
I'm too seeing this in my application. I recently upgraded from Ember 1.13.10 all the way up to Ember 2.4.3 and now some of the routes are broken, especially when reloading them... |
What i've found, is that if you run something in the controller (like |
Any recommendations for fixes using the remote paginated API? |
+1 thought I'd also share my specific problem and workaround 😁 In our application, we have two date search boxes, ‘From Date’ and ‘To Date.’ When a user visits the page, 'From Date' and 'To Date' display the date range of our applications' custom current cycle. The 'From Date' and 'To Date' change based on the current date thus, the calculations are done in a service. 'From Date' and 'To Date' are also query parameters, set in the controller, for export purposes and because our users use this detailed link. As a workaround, I used the setupController in a route and set the 'from' and 'to’ to grab the calculations from the service.
|
before mapping all the queryParams. In certain scenarios, if query params are shared through a service, bound query params will fire the _qpDelegate while transitioning to a new controller if the property has been modified from it's default value. Loosely related to - emberjs#11592 Based on a previous PR - emberjs#13981
before mapping all the queryParams. In certain scenarios, if query params are shared through a service, bound query params will fire the _qpDelegate while transitioning to a new controller if the property has been modified from it's default value. Loosely related to - emberjs#11592 Based on a previous PR - emberjs#13981
before mapping all the queryParams. In certain scenarios, if query params are shared through a service, bound query params will fire the _qpDelegate while transitioning to a new controller if the property has been modified from it's default value. Loosely related to - #11592 Based on a previous PR - #13981 (cherry picked from commit 8f1db44)
I just updated our app to try the latest Canary and it appears to break query params for us.
We're seeing:
Here's the backtrace etc…
I'll see if I can reproduce in a smaller app asap.
The text was updated successfully, but these errors were encountered: