fix(router): make routerLinkActive work with query params which contain arrays (#22666)

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 #22223

PR Close #22666
This commit is contained in:
Zaid Al-Omari 2018-03-09 03:29:55 +04:00 committed by Matias Niemelä
parent c8ea99f9da
commit 828e5c1f79
4 changed files with 48 additions and 6 deletions

View File

@ -25,7 +25,7 @@ export const PRIMARY_OUTLET = 'primary';
* @publicApi * @publicApi
*/ */
export type Params = { export type Params = {
[key: string]: any [key: string]: any;
}; };
/** /**

View File

@ -7,7 +7,7 @@
*/ */
import {PRIMARY_OUTLET, ParamMap, Params, convertToParamMap} from './shared'; import {PRIMARY_OUTLET, ParamMap, Params, convertToParamMap} from './shared';
import {forEach, shallowEqual} from './utils/collection'; import {equalArraysOrString, forEach, shallowEqual} from './utils/collection';
export function createEmptyUrlTree() { export function createEmptyUrlTree() {
return new UrlTree(new UrlSegmentGroup([], {}), {}, null); return new UrlTree(new UrlSegmentGroup([], {}), {}, null);
@ -41,7 +41,7 @@ function equalSegmentGroups(container: UrlSegmentGroup, containee: UrlSegmentGro
function containsQueryParams(container: Params, containee: Params): boolean { function containsQueryParams(container: Params, containee: Params): boolean {
// TODO: This does not handle array params correctly. // TODO: This does not handle array params correctly.
return Object.keys(containee).length <= Object.keys(container).length && 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 { function containsSegmentGroup(container: UrlSegmentGroup, containee: UrlSegmentGroup): boolean {

View File

@ -10,7 +10,7 @@ import {NgModuleFactory, ɵisObservable as isObservable, ɵisPromise as isPromis
import {Observable, from, of } from 'rxjs'; import {Observable, from, of } from 'rxjs';
import {concatAll, last as lastValue, map} from 'rxjs/operators'; import {concatAll, last as lastValue, map} from 'rxjs/operators';
import {PRIMARY_OUTLET} from '../shared'; import {PRIMARY_OUTLET, Params} from '../shared';
export function shallowEqualArrays(a: any[], b: any[]): boolean { export function shallowEqualArrays(a: any[], b: any[]): boolean {
if (a.length !== b.length) return false; if (a.length !== b.length) return false;
@ -20,7 +20,7 @@ export function shallowEqualArrays(a: any[], b: any[]): boolean {
return true; return true;
} }
export function shallowEqual(a: {[x: string]: any}, b: {[x: string]: any}): boolean { export function shallowEqual(a: Params, b: Params): boolean {
// Casting Object.keys return values to include `undefined` as there are some cases // Casting Object.keys return values to include `undefined` as there are some cases
// in IE 11 where this can happen. Cannot provide a test because the behavior only // in IE 11 where this can happen. Cannot provide a test because the behavior only
// exists in certain circumstances in IE 11, therefore doing this cast ensures the // exists in certain circumstances in IE 11, therefore doing this cast ensures the
@ -33,13 +33,25 @@ export function shallowEqual(a: {[x: string]: any}, b: {[x: string]: any}): bool
let key: string; let key: string;
for (let i = 0; i < k1.length; i++) { for (let i = 0; i < k1.length; i++) {
key = k1[i]; key = k1[i];
if (a[key] !== b[key]) { if (!equalArraysOrString(a[key], b[key])) {
return false; return false;
} }
} }
return true; return true;
} }
/**
* Test equality for arrays of strings or a string.
*/
export function equalArraysOrString(a: string | string[], b: string | string[]) {
if (Array.isArray(a) && Array.isArray(b)) {
if (a.length != b.length) return false;
return a.every(aItem => b.indexOf(aItem) > -1);
} else {
return a === b;
}
}
/** /**
* Flattens single-level nested arrays. * Flattens single-level nested arrays.
*/ */

View File

@ -41,6 +41,24 @@ describe('UrlTree', () => {
expect(containsTree(t1, t2, true)).toBe(true); 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', () => {
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', () => {
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', () => { it('should return false when queryParams are not the same', () => {
const t1 = serializer.parse('/one/two?test=1&page=5'); const t1 = serializer.parse('/one/two?test=1&page=5');
const t2 = serializer.parse('/one/two?test=1'); const t2 = serializer.parse('/one/two?test=1');
@ -133,6 +151,18 @@ describe('UrlTree', () => {
expect(containsTree(t1, t2, false)).toBe(false); expect(containsTree(t1, t2, false)).toBe(false);
}); });
it('should return true when container has array params but containee does not have', () => {
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', () => {
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', () => { it('should return false when containee has different queryParams', () => {
const t1 = serializer.parse('/one/two?page=5'); const t1 = serializer.parse('/one/two?page=5');
const t2 = serializer.parse('/one/two?test=1'); const t2 = serializer.parse('/one/two?test=1');