Merge pull request #198 from alexspeller/fix-queryParamsOnly-property…

…-on-transition Fix queryParamsOnly being correct on transitions.
tildeio · Jan 17, 2017 · b4419b7 · b4419b7
2 parents ca5cb2a + 9e5440b
commit b4419b7
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 0 deletions.
diff --git a/lib/router/router.js b/lib/router/router.js
@@ -47,6 +47,7 @@ function getTransitionByIntent(intent, isIntermediate) {
     if (queryParamChangelist) {
       newTransition = this.queryParamsTransition(queryParamChangelist, wasTransitioning, oldState, newState);
       if (newTransition) {
+        newTransition.queryParamsOnly = true;
         return newTransition;
       }
     }
@@ -63,6 +64,12 @@ function getTransitionByIntent(intent, isIntermediate) {
   // Create a new transition to the destination route.
   newTransition = new Transition(this, intent, newState, undefined, this.activeTransition);
 
+  // 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();
@@ -772,6 +779,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

diff --git a/test/tests/query_params_test.js b/test/tests/query_params_test.js
@@ -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);