fix(router): make routerLinkActive work with query params which conta…

…in arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223

packages/router/src/url_tree.ts

@@ -7,7 +7,7 @@
  */
 
 import {PRIMARY_OUTLET, ParamMap, convertToParamMap} from './shared';
-import {forEach, shallowEqual} from './utils/collection';
+import {equalArraysOrString, forEach, shallowEqual} from './utils/collection';
 
 export function createEmptyUrlTree() {
   return new UrlTree(new UrlSegmentGroup([], {}), {}, null);
@@ -24,7 +24,8 @@ export function containsTree(container: UrlTree, containee: UrlTree, exact: bool
 }
 
 function equalQueryParams(
-    container: {[k: string]: string}, containee: {[k: string]: string}): boolean {
+    container: {[k: string]: string | string[]},
+    containee: {[k: string]: string | string[]}): boolean {
   return shallowEqual(container, containee);
 }
 
@@ -39,9 +40,10 @@ function equalSegmentGroups(container: UrlSegmentGroup, containee: UrlSegmentGro
 }
 
 function containsQueryParams(
-    container: {[k: string]: string}, containee: {[k: string]: string}): boolean {
+    container: {[k: string]: string | string[]},
+    containee: {[k: string]: string | string[]}): boolean {
   return Object.keys(containee).length <= Object.keys(container).length &&
-      Object.keys(containee).every(key => containee[key] === container[key]);
+      Object.keys(containee).every(key => equalArraysOrString(container[key], containee[key]));
 }
 
 function containsSegmentGroup(container: UrlSegmentGroup, containee: UrlSegmentGroup): boolean {
@@ -111,7 +113,7 @@ export class UrlTree {
       /** The root segment group of the URL tree */
       public root: UrlSegmentGroup,
       /** The query params of the URL */
-      public queryParams: {[key: string]: string},
+      public queryParams: {[key: string]: string | string[]},
       /** The fragment of the URL */
       public fragment: string|null) {}
 

packages/router/src/utils/collection.ts

@@ -25,7 +25,8 @@ export function shallowEqualArrays(a: any[], b: any[]): boolean {
   return true;
 }
 
-export function shallowEqual(a: {[x: string]: any}, b: {[x: string]: any}): boolean {
+export function shallowEqual(
+    a: {[x: string]: string | Array<string>}, b: {[x: string]: string | Array<string>}): boolean {
   const k1 = Object.keys(a);
   const k2 = Object.keys(b);
   if (k1.length != k2.length) {
@@ -34,13 +35,25 @@ export function shallowEqual(a: {[x: string]: any}, b: {[x: string]: any}): bool
   let key: string;
   for (let i = 0; i < k1.length; i++) {
     key = k1[i];
-    if (a[key] !== b[key]) {
+    if (!equalArraysOrString(a[key], b[key])) {
       return false;
     }
   }
   return true;
 }
 
+/**
+ * Test equality for arrays of strings or a string.
+ */
+export function equalArraysOrString(a: string | Array<String>, b: string | Array<String>) {
+  if (a instanceof Array && b instanceof Array) {
+    if (a.length != b.length) return false;
+    return a.every(aItem => b.indexOf(aItem) > -1);
+  } else {
+    return a === b;
+  }
+}
+
 /**
  * Flattens single-level nested arrays.
  */

packages/router/test/url_tree.spec.ts

@@ -41,6 +41,26 @@ describe('UrlTree', () => {
         expect(containsTree(t1, t2, true)).toBe(true);
       });
 
+      it('should return true when queryParams are the same but with diffrent order', () => {
+        const t1 = serializer.parse('/one/two?test=1&page=5');
+        const t2 = serializer.parse('/one/two?page=5&test=1');
+        expect(containsTree(t1, t2, true)).toBe(true);
+      });
+
+      it('should return true when queryParams contains array params that are the same', () => {
+        debugger;
+        const t1 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=6');
+        const t2 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=6');
+        expect(containsTree(t1, t2, true)).toBe(true);
+      });
+
+      it('should return false when queryParams contains array params but are not the same', () => {
+        debugger;
+        const t1 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=6');
+        const t2 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=7');
+        expect(containsTree(t1, t2, false)).toBe(false);
+      });
+
       it('should return false when queryParams are not the same', () => {
         const t1 = serializer.parse('/one/two?test=1&page=5');
         const t2 = serializer.parse('/one/two?test=1');
@@ -133,6 +153,20 @@ describe('UrlTree', () => {
         expect(containsTree(t1, t2, false)).toBe(false);
       });
 
+      it('should return true when container has array params but containee does not have', () => {
+        debugger;
+        const t1 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=6');
+        const t2 = serializer.parse('/one/two?test=a&test=b');
+        expect(containsTree(t1, t2, false)).toBe(true);
+      });
+
+      it('should return false when containee has array params but container does not have', () => {
+        debugger;
+        const t1 = serializer.parse('/one/two?test=a&test=b');
+        const t2 = serializer.parse('/one/two?test=a&test=b&pages=5&pages=6');
+        expect(containsTree(t1, t2, false)).toBe(false);
+      });
+
       it('should return false when containee has different queryParams', () => {
         const t1 = serializer.parse('/one/two?page=5');
         const t2 = serializer.parse('/one/two?test=1');