Skip to content

Commit 6639900

Browse files
raytileyStanley Stuart
authored andcommitted
Normalize query params when merging in from active transition
This merges in queryParams from an active transition in such a way that `prepareQueryParams` won't duplicate them. It also won't merge in the qp if it is already int he qp changed list, for example if it's already been set during route setup by a call to `controller.set('qpProp', 'someValue')`. Fixing these things with my current test jsbin also recreated another issue that has been biting people and that is that a call to `transitionTo` creates an aborted transition that calls `didTransition` with an empty set of handlerInfos. This would cause the `updatePaths` method to fail since it couldn't calculate the paths without the handler infos. We now bail early from `updatePaths` it is called with an empty array of handlerInfos. I think this is fine because the aborted transition is followed by a successful transition that will update the paths appropriatly. This gets the active queryParams and the provided using the same key `controller:prop` so that the calls to `assign` override as expected. This fixes issues where the QP doesn't update properly on refresh because the merged value conflicts with the provided value.
1 parent 81102a1 commit 6639900

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

packages/ember-routing/lib/system/router.js

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,8 @@ const EmberRouter = EmberObject.extend(Evented, {
674674
assign(queryParams, this.router.activeTransition.queryParams);
675675
}
676676

677+
this._processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams);
678+
677679
assign(queryParams, _queryParams);
678680
this._prepareQueryParams(targetRouteName, models, queryParams);
679681

@@ -685,6 +687,27 @@ const EmberRouter = EmberObject.extend(Evented, {
685687
return transition;
686688
},
687689

690+
_processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams) {
691+
// merge in any queryParams from the active transition which could include
692+
// queryparams from the url on initial load.
693+
if (!this.router.activeTransition) { return; }
694+
695+
var unchangedQPs = {};
696+
var qpUpdates = this._qpUpdates || {};
697+
for (var key in this.router.activeTransition.queryParams) {
698+
if (!qpUpdates[key]) {
699+
unchangedQPs[key] = this.router.activeTransition.queryParams[key];
700+
}
701+
}
702+
703+
// We need to fully scope query params so that we can create one object
704+
// that represetns both pased in query params and ones that arent' changed
705+
// from the actice transition
706+
this._fullyScopeQueryParams(targetRouteName, models, _queryParams);
707+
this._fullyScopeQueryParams(targetRouteName, models, unchangedQPs);
708+
assign(queryParams, unchangedQPs);
709+
},
710+
688711
_prepareQueryParams(targetRouteName, models, queryParams) {
689712
this._hydrateUnsuppliedQueryParams(targetRouteName, models, queryParams);
690713
this._serializeQueryParams(targetRouteName, queryParams);
@@ -729,6 +752,32 @@ const EmberRouter = EmberObject.extend(Evented, {
729752
};
730753
},
731754

755+
_fullyScopeQueryParams(leafRouteName, contexts, queryParams) {
756+
var state = calculatePostTransitionState(this, leafRouteName, contexts);
757+
var handlerInfos = state.handlerInfos;
758+
stashParamNames(this, handlerInfos);
759+
760+
for (var i = 0, len = handlerInfos.length; i < len; ++i) {
761+
var route = handlerInfos[i].handler;
762+
var qpMeta = get(route, '_qp');
763+
764+
for (var j = 0, qpLen = qpMeta.qps.length; j < qpLen; ++j) {
765+
var qp = qpMeta.qps[j];
766+
767+
var presentProp = qp.prop in queryParams && qp.prop ||
768+
qp.scopedPropertyName in queryParams && qp.scopedPropertyName ||
769+
qp.urlKey in queryParams && qp.urlKey;
770+
771+
if (presentProp) {
772+
if (presentProp !== qp.scopedPropertyName) {
773+
queryParams[qp.scopedPropertyName] = queryParams[presentProp];
774+
delete queryParams[presentProp];
775+
}
776+
}
777+
}
778+
}
779+
},
780+
732781
_hydrateUnsuppliedQueryParams(leafRouteName, contexts, queryParams) {
733782
let state = calculatePostTransitionState(this, leafRouteName, contexts);
734783
let handlerInfos = state.handlerInfos;
@@ -743,7 +792,8 @@ const EmberRouter = EmberObject.extend(Evented, {
743792
let qp = qpMeta.qps[j];
744793

745794
let presentProp = qp.prop in queryParams && qp.prop ||
746-
qp.scopedPropertyName in queryParams && qp.scopedPropertyName;
795+
qp.scopedPropertyName in queryParams && qp.scopedPropertyName ||
796+
qp.urlKey in queryParams && qp.urlKey;
747797

748798
if (presentProp) {
749799
if (presentProp !== qp.scopedPropertyName) {
@@ -999,6 +1049,8 @@ function calculatePostTransitionState(emberRouter, leafRouteName, contexts) {
9991049

10001050
function updatePaths(router) {
10011051
let infos = router.router.currentHandlerInfos;
1052+
if (infos.length === 0) { return; }
1053+
10021054
let path = EmberRouter._routePath(infos);
10031055
let currentRouteName = infos[infos.length - 1].name;
10041056

packages/ember/tests/routing/query_params_test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,6 +2490,43 @@ if (isEnabled('ember-routing-route-configured-query-params')) {
24902490
bootApplication();
24912491
});
24922492

2493+
QUnit.test('queryParams are updated when a controller property is set and the route is refreshed. Issue #13263 ', function() {
2494+
Ember.TEMPLATES.application = compile(
2495+
'<button id="test-button" {{action \'increment\'}}>Increment</button>' +
2496+
'<span id="test-value">{{foo}}</span>' +
2497+
'{{outlet}}'
2498+
);
2499+
App.ApplicationController = Controller.extend({
2500+
queryParams: ['foo'],
2501+
foo: 1,
2502+
actions: {
2503+
increment: function() {
2504+
this.incrementProperty('foo');
2505+
this.send('refreshRoute');
2506+
}
2507+
}
2508+
});
2509+
2510+
App.ApplicationRoute = Route.extend({
2511+
actions: {
2512+
refreshRoute: function() {
2513+
this.refresh();
2514+
}
2515+
}
2516+
});
2517+
2518+
startingURL = '/';
2519+
bootApplication();
2520+
equal(jQuery('#test-value').text().trim(), '1');
2521+
equal(router.get('location.path'), '/', 'url is correct');
2522+
run(jQuery('#test-button'), 'click');
2523+
equal(jQuery('#test-value').text().trim(), '2');
2524+
equal(router.get('location.path'), '/?foo=2', 'url is correct');
2525+
run(jQuery('#test-button'), 'click');
2526+
equal(jQuery('#test-value').text().trim(), '3');
2527+
equal(router.get('location.path'), '/?foo=3', 'url is correct');
2528+
});
2529+
24932530
QUnit.test('Use Ember.get to retrieve query params \'refreshModel\' configuration', function() {
24942531
expect(6);
24952532
App.ApplicationController = Controller.extend({

0 commit comments

Comments
 (0)