fix(RouterLink): ignore optional parameters when checking for active routes

vicb · vicb · commit 5e2bc5c593f1 · 2016-04-07T19:41:14.000Z
fixes angular#6459 Closes angular#7834
diff --git a/modules/angular2/src/router/instruction.ts b/modules/angular2/src/router/instruction.ts
@@ -310,7 +310,7 @@ export class ComponentInstruction {
    */
   constructor(public urlPath: string, public urlParams: string[], data: RouteData,
               public componentType, public terminal: boolean, public specificity: string,
-              public params: {[key: string]: string} = null) {
+              public params: {[key: string]: string} = null, public routeName: string) {
     this.routeData = isPresent(data) ? data : BLANK_ROUTE_DATA;
   }
 }
diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts
@@ -135,12 +135,33 @@ export class Router {
    */
   isRouteActive(instruction: Instruction): boolean {
     var router: Router = this;
+
+    if (isBlank(this.currentInstruction)) {
+      return false;
+    }
+
+    // `instruction` corresponds to the root router
     while (isPresent(router.parent) && isPresent(instruction.child)) {
       router = router.parent;
       instruction = instruction.child;
     }
-    return isPresent(this.currentInstruction) &&
-           this.currentInstruction.component == instruction.component;
+
+    if (isBlank(instruction.component) || isBlank(this.currentInstruction.component) ||
+        this.currentInstruction.component.routeName != instruction.component.routeName) {
+      return false;
+    }
+
+    let paramEquals = true;
+
+    if (isPresent(this.currentInstruction.component.params)) {
+      StringMapWrapper.forEach(instruction.component.params, (value, key) => {
+        if (this.currentInstruction.component.params[key] !== value) {
+          paramEquals = false;
+        }
+      });
+    }
+
+    return paramEquals;
   }
 
 
diff --git a/modules/angular2/src/router/rules/rule_set.ts b/modules/angular2/src/router/rules/rule_set.ts
@@ -59,7 +59,7 @@ export class RuleSet {
     if (config instanceof AuxRoute) {
       handler = new SyncRouteHandler(config.component, config.data);
       let routePath = this._getRoutePath(config);
-      let auxRule = new RouteRule(routePath, handler);
+      let auxRule = new RouteRule(routePath, handler, config.name);
       this.auxRulesByPath.set(routePath.toString(), auxRule);
       if (isPresent(config.name)) {
         this.auxRulesByName.set(config.name, auxRule);
@@ -85,7 +85,7 @@ export class RuleSet {
       useAsDefault = isPresent(config.useAsDefault) && config.useAsDefault;
     }
     let routePath = this._getRoutePath(config);
-    let newRule = new RouteRule(routePath, handler);
+    let newRule = new RouteRule(routePath, handler, config.name);
 
     this._assertNoHashCollision(newRule.hash, config.path);
 
diff --git a/modules/angular2/src/router/rules/rules.ts b/modules/angular2/src/router/rules/rules.ts
@@ -69,7 +69,8 @@ export class RouteRule implements AbstractRule {
 
   // TODO: cache component instruction instances by params and by ParsedUrl instance
 
-  constructor(private _routePath: RoutePath, public handler: RouteHandler) {
+  constructor(private _routePath: RoutePath, public handler: RouteHandler,
+              private _routeName: string) {
     this.specificity = this._routePath.specificity;
     this.hash = this._routePath.hash;
     this.terminal = this._routePath.terminal;
@@ -112,7 +113,7 @@ export class RouteRule implements AbstractRule {
     }
     var instruction =
         new ComponentInstruction(urlPath, urlParams, this.handler.data, this.handler.componentType,
-                                 this.terminal, this.specificity, params);
+                                 this.terminal, this.specificity, params, this._routeName);
     this._cache.set(hashKey, instruction);
 
     return instruction;
diff --git a/modules/angular2/test/router/directives/router_link_spec.ts b/modules/angular2/test/router/directives/router_link_spec.ts
@@ -34,7 +34,7 @@ import {DOM} from 'angular2/src/platform/dom/dom_adapter';
 import {ResolvedInstruction} from 'angular2/src/router/instruction';
 
 let dummyInstruction = new ResolvedInstruction(
-    new ComponentInstruction('detail', [], null, null, true, '0'), null, {});
+    new ComponentInstruction('detail', [], null, null, true, '0', null, 'Detail'), null, {});
 
 export function main() {
   describe('routerLink directive', function() {
diff --git a/modules/angular2/test/router/integration/router_link_spec.ts b/modules/angular2/test/router/integration/router_link_spec.ts
@@ -259,7 +259,7 @@ export function main() {
 
                    async.done();
                  });
-                 router.navigateByUrl('/better-child');
+                 router.navigateByUrl('/better-child?extra=0');
                });
          }));
 
@@ -300,7 +300,7 @@ export function main() {
 
                    async.done();
                  });
-                 router.navigateByUrl('/child-with-grandchild/grandchild');
+                 router.navigateByUrl('/child-with-grandchild/grandchild?extra=0');
                });
          }));
 

Original file line number	Diff line number	Diff line change
`@@ -310,7 +310,7 @@ export class ComponentInstruction {`
`310`	`310`	`*/`
`311`	`311`	`constructor(public urlPath: string, public urlParams: string[], data: RouteData,`
`312`	`312`	`public componentType, public terminal: boolean, public specificity: string,`
`313`		`- public params: {[key: string]: string} = null) {`
	`313`	`+ public params: {[key: string]: string} = null, public routeName: string) {`
`314`	`314`	`this.routeData = isPresent(data) ? data : BLANK_ROUTE_DATA;`
`315`	`315`	`}`
`316`	`316`	`}`