-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
Query-Params Lifecycle #192
Conversation
6a1ace6
to
c118096
Compare
c118096
to
afce762
Compare
Related: emberjs/ember.js#9819, emberjs/ember.js#11592. See also http://stackoverflow.com/q/35192211/1190, which works in an action handler, but not in a route hook like |
I've done some monkey-patching to fix some of these things. In Ember 1.13, I had the following to ensure that all query-params were synced // route:application
serializeQueryParam(value, key, ...args) {
value = value || this.controllerFor('application').get(key);
return this._super(value, key, ...args);
},
model() {
const controller = this.controllerFor(this.routeName);
controller.getWithDefault('queryParams', []).forEach(function(key) {
if (controller.get(key) != null) return;
controller.set(key, transition.queryParams[key]);
});
} But that broke on Ember 2.x due to emberjs/ember.js#11592. I changed the model(params, { queryParams }) {
return queryParams;
},
afterModel(model, transition) {
transition.send('fixQueryParams', transition.queryParams);
},
setupController(controller, model) {
model = _.pick(model, [ 'valid', 'queryParams' ]);
return this._super(controller, model);
},
actions: {
fixQueryParams(queryParams) {
let needsTransition = false;
// do some munging of queryParams; if any munging, set needsTransition
if (needsTransition) {
Ember.run.next(this, 'replaceWith', { queryParams });
}
}
} I couldn't do the |
I think it is great to have this use case in mind and to start fleshing out what the constraints are, but this seems more like it belongs as an issue while we flesh it out into an actual concrete proposal. Thoughts? |
No objections. I'll reframe it as an issue on emberjs/ember.js and reference this RFC. |
Replacing with emberjs/ember.js#14785 for now. |
Rendered