From b90aac0d43a2cc58c4cdbfdf3ee35e43c88c03c3 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 15 Nov 2019 12:45:37 -0800 Subject: [PATCH] Revert "fix(router): make routerLinkActive work with query params which contain arrays (#22666)" (#33861) This reverts commit b30bb8dd91c61684f16664e6ae6589113737a76d. Reason: breaks internal g3 project. PR Close #33861 --- packages/router/src/shared.ts | 2 +- packages/router/src/url_tree.ts | 4 ++-- packages/router/src/utils/collection.ts | 18 +++------------ packages/router/test/url_tree.spec.ts | 30 ------------------------- 4 files changed, 6 insertions(+), 48 deletions(-) diff --git a/packages/router/src/shared.ts b/packages/router/src/shared.ts index 51c861cc8f..8a0c4c3b96 100644 --- a/packages/router/src/shared.ts +++ b/packages/router/src/shared.ts @@ -25,7 +25,7 @@ export const PRIMARY_OUTLET = 'primary'; * @publicApi */ export type Params = { - [key: string]: any; + [key: string]: any }; /** diff --git a/packages/router/src/url_tree.ts b/packages/router/src/url_tree.ts index 94fc9f0b37..3ea9dcf231 100644 --- a/packages/router/src/url_tree.ts +++ b/packages/router/src/url_tree.ts @@ -7,7 +7,7 @@ */ import {PRIMARY_OUTLET, ParamMap, Params, convertToParamMap} from './shared'; -import {equalArraysOrString, forEach, shallowEqual} from './utils/collection'; +import {forEach, shallowEqual} from './utils/collection'; export function createEmptyUrlTree() { return new UrlTree(new UrlSegmentGroup([], {}), {}, null); @@ -41,7 +41,7 @@ function equalSegmentGroups(container: UrlSegmentGroup, containee: UrlSegmentGro function containsQueryParams(container: Params, containee: Params): boolean { // TODO: This does not handle array params correctly. return Object.keys(containee).length <= Object.keys(container).length && - Object.keys(containee).every(key => equalArraysOrString(container[key], containee[key])); + Object.keys(containee).every(key => containee[key] === container[key]); } function containsSegmentGroup(container: UrlSegmentGroup, containee: UrlSegmentGroup): boolean { diff --git a/packages/router/src/utils/collection.ts b/packages/router/src/utils/collection.ts index 74fbc81420..d6c47dda19 100644 --- a/packages/router/src/utils/collection.ts +++ b/packages/router/src/utils/collection.ts @@ -10,7 +10,7 @@ import {NgModuleFactory, ɵisObservable as isObservable, ɵisPromise as isPromis import {Observable, from, of } from 'rxjs'; import {concatAll, last as lastValue, map} from 'rxjs/operators'; -import {PRIMARY_OUTLET, Params} from '../shared'; +import {PRIMARY_OUTLET} from '../shared'; export function shallowEqualArrays(a: any[], b: any[]): boolean { if (a.length !== b.length) return false; @@ -20,7 +20,7 @@ export function shallowEqualArrays(a: any[], b: any[]): boolean { return true; } -export function shallowEqual(a: Params, b: Params): boolean { +export function shallowEqual(a: {[x: string]: any}, b: {[x: string]: any}): boolean { // 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 // exists in certain circumstances in IE 11, therefore doing this cast ensures the @@ -33,25 +33,13 @@ export function shallowEqual(a: Params, b: Params): boolean { let key: string; for (let i = 0; i < k1.length; i++) { key = k1[i]; - if (!equalArraysOrString(a[key], b[key])) { + if (a[key] !== b[key]) { return false; } } 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. */ diff --git a/packages/router/test/url_tree.spec.ts b/packages/router/test/url_tree.spec.ts index d6a31145b4..2b46cb1add 100644 --- a/packages/router/test/url_tree.spec.ts +++ b/packages/router/test/url_tree.spec.ts @@ -41,24 +41,6 @@ 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', () => { - 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', () => { const t1 = serializer.parse('/one/two?test=1&page=5'); const t2 = serializer.parse('/one/two?test=1'); @@ -151,18 +133,6 @@ describe('UrlTree', () => { 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', () => { const t1 = serializer.parse('/one/two?page=5'); const t2 = serializer.parse('/one/two?test=1');