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

Fix queryParamsOnly being correct on transitions. #198

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions lib/router/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function getTransitionByIntent(intent, isIntermediate) {
if (queryParamChangelist) {
newTransition = this.queryParamsTransition(queryParamChangelist, wasTransitioning, oldState, newState);
if (newTransition) {
newTransition.queryParamsOnly = true;
return newTransition;
}
}
Expand All @@ -61,6 +62,12 @@ function getTransitionByIntent(intent, isIntermediate) {
// Create a new transition to the destination route.
newTransition = new Transition(this, intent, newState);

// transition is to same route with same params, only query params differ.
// not caught above probably because refresh() has been used
if ( handlerInfosSameExceptQueryParams(newState.handlerInfos, oldState.handlerInfos ) ) {
newTransition.queryParamsOnly = true;
}

// Abort and usurp any previously active transition.
if (this.activeTransition) {
this.activeTransition.abort();
Expand Down Expand Up @@ -750,6 +757,50 @@ function handlerInfosEqual(handlerInfos, otherHandlerInfos) {
return true;
}

function handlerInfosSameExceptQueryParams(handlerInfos, otherHandlerInfos) {
if (handlerInfos.length !== otherHandlerInfos.length) {
return false;
}

for (var i = 0, len = handlerInfos.length; i < len; ++i) {
if (handlerInfos[i].name !== otherHandlerInfos[i].name) {
return false;
}

if (!paramsEqual(handlerInfos[i].params, otherHandlerInfos[i].params)) {
return false;
}
}
return true;

}

function paramsEqual(params, otherParams) {
if (!params && !otherParams) {
return true;
} else if (!params && !!otherParams || !!params && !otherParams) {
// one is falsy but other is not;
return false;
}
var keys = Object.keys(params);
var otherKeys = Object.keys(otherParams);

if (keys.length !== otherKeys.length) {
return false;
}

for (var i = 0, len = keys.length; i < len; ++i) {
var key = keys[i];

if ( params[key] !== otherParams[key] ) {
return false;
}
}

return true;

}

function finalizeQueryParamChange(router, resolvedHandlers, newQueryParams, transition) {
// We fire a finalizeQueryParamChange event which
// gives the new route hierarchy a chance to tell
Expand Down
55 changes: 55 additions & 0 deletions test/tests/query_params_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,61 @@ test("transitioning between routes fires a queryParamsDidChange event", function

});


test("Refreshing the route when changing only query params should correctly set queryParamsOnly", function(assert) {
assert.expect(10);

var initialTransition = true;

handlers.index = {
events: {
finalizeQueryParamChange: function(params, finalParams, transition) {
if (initialTransition) {
assert.notOk(transition.queryParamsOnly, 'should not be query params only transition');
initialTransition = false;
} else {
assert.ok(transition.queryParamsOnly, 'should be query params only transition');
}
},

queryParamsDidChange: function() {
router.refresh();
}
}
};

handlers.child = {
events: {
finalizeQueryParamChange: function(params, finalParams, transition) {
assert.notOk(transition.queryParamsOnly, 'should be normal transition');
return true;
},
}
};

var transition = transitionTo(router, '/index');
assert.notOk(transition.queryParamsOnly, "Initial transition is not query params only transition");

transition = transitionTo(router, '/index?foo=123');

assert.ok(transition.queryParamsOnly, "Second transition with updateURL intent is query params only");

transition = router.replaceWith('/index?foo=456');
flushBackburner();

assert.ok(transition.queryParamsOnly, "Third transition with replaceURL intent is query params only");


transition = transitionTo(router, '/parent/child?foo=789');
assert.notOk(transition.queryParamsOnly, "Fourth transition with transtionTo intent is not query params only");

transition = transitionTo(router, '/parent/child?foo=901');
assert.ok(transition.queryParamsOnly, "Firth transition with transtionTo intent is query params only");

transition = transitionTo(router, '/index?foo=123');
assert.notOk(transition.queryParamsOnly, "Firth transition with transtionTo intent is not query params only");
});

test("a handler can opt into a full-on transition by calling refresh", function(assert) {
assert.expect(3);

Expand Down