Revert "fix(router): Respect custom UrlSerializer handling of query parameters (angular#64449)"

This reverts commit 46ae034.

Author: thePunderWoman

goldens/public-api/router/index.api.md

@@ -205,7 +205,7 @@ export type ComponentInputBindingFeature = RouterFeature<RouterFeatureKind.Compo
 export function convertToParamMap(params: Params): ParamMap;
 
 // @public
-export function createUrlTreeFromSnapshot(relativeTo: ActivatedRouteSnapshot, commands: readonly any[], queryParams?: Params | null, fragment?: string | null, urlSerializer?: DefaultUrlSerializer): UrlTree;
+export function createUrlTreeFromSnapshot(relativeTo: ActivatedRouteSnapshot, commands: readonly any[], queryParams?: Params | null, fragment?: string | null): UrlTree;
 
 // @public
 export type Data = {

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -931,7 +931,6 @@
       "noop2",
       "noop3",
       "normalizeQueryParams",
-      "normalizeQueryParams2",
       "notFoundValueOrThrow",
       "observable",
       "observeOn",

packages/router/src/create_url_tree.ts

@@ -11,15 +11,7 @@ import {ɵRuntimeError as RuntimeError} from '@angular/core';
 import {RuntimeErrorCode} from './errors';
 import {ActivatedRouteSnapshot} from './router_state';
 import {Params, PRIMARY_OUTLET} from './shared';
-import {
-  createRoot,
-  DefaultUrlSerializer,
-  squashSegmentGroup,
-  UrlSegment,
-  UrlSegmentGroup,
-  UrlSerializer,
-  UrlTree,
-} from './url_tree';
+import {createRoot, squashSegmentGroup, UrlSegment, UrlSegmentGroup, UrlTree} from './url_tree';
 import {last, shallowEqual} from './utils/collection';
 
 /**
@@ -36,9 +28,6 @@ import {last, shallowEqual} from './utils/collection';
  * @param queryParams The query parameters for the `UrlTree`. `null` if the `UrlTree` does not have
  *     any query parameters.
  * @param fragment The fragment for the `UrlTree`. `null` if the `UrlTree` does not have a fragment.
- * @param urlSerializer The `UrlSerializer` to use for handling query parameter normalization.
- * You should provide your application's custom `UrlSerializer` if one is configured to parse and
- * serialize query parameter values to and from objects other than strings/string arrays.
  *
  * @usageNotes
  *
@@ -81,16 +70,9 @@ export function createUrlTreeFromSnapshot(
   commands: readonly any[],
   queryParams: Params | null = null,
   fragment: string | null = null,
-  urlSerializer = new DefaultUrlSerializer(),
 ): UrlTree {
   const relativeToUrlSegmentGroup = createSegmentGroupFromRoute(relativeTo);
-  return createUrlTreeFromSegmentGroup(
-    relativeToUrlSegmentGroup,
-    commands,
-    queryParams,
-    fragment,
-    urlSerializer,
-  );
+  return createUrlTreeFromSegmentGroup(relativeToUrlSegmentGroup, commands, queryParams, fragment);
 }
 
 export function createSegmentGroupFromRoute(route: ActivatedRouteSnapshot): UrlSegmentGroup {
@@ -121,7 +103,6 @@ export function createUrlTreeFromSegmentGroup(
   commands: readonly any[],
   queryParams: Params | null,
   fragment: string | null,
-  urlSerializer: UrlSerializer,
 ): UrlTree {
   let root = relativeTo;
   while (root.parent) {
@@ -131,20 +112,20 @@ export function createUrlTreeFromSegmentGroup(
   // `UrlSegmentGroup`. All we need to do is update the `queryParams` and `fragment` without
   // applying any other logic.
   if (commands.length === 0) {
-    return tree(root, root, root, queryParams, fragment, urlSerializer);
+    return tree(root, root, root, queryParams, fragment);
   }
 
   const nav = computeNavigation(commands);
 
   if (nav.toRoot()) {
-    return tree(root, root, new UrlSegmentGroup([], {}), queryParams, fragment, urlSerializer);
+    return tree(root, root, new UrlSegmentGroup([], {}), queryParams, fragment);
   }
 
   const position = findStartingPositionForTargetGroup(nav, root, relativeTo);
   const newSegmentGroup = position.processChildren
     ? updateSegmentGroupChildren(position.segmentGroup, position.index, nav.commands)
     : updateSegmentGroup(position.segmentGroup, position.index, nav.commands);
-  return tree(root, position.segmentGroup, newSegmentGroup, queryParams, fragment, urlSerializer);
+  return tree(root, position.segmentGroup, newSegmentGroup, queryParams, fragment);
 }
 
 function isMatrixParams(command: any): boolean {
@@ -159,43 +140,18 @@ function isCommandWithOutlets(command: any): command is {outlets: {[key: string]
   return typeof command === 'object' && command != null && command.outlets;
 }
 
-/**
- * Normalizes a query parameter value by using the `UrlSerializer` to serialize then parse the value.
- *
- * This ensures that the value is consistent between parsing a URL in the browser on a fresh page load (or page refresh)
- * and a navigation where the query parameter value is passed directly to the router.
- *
- * This also allows custom `UrlSerializer` implementations to define how query parameter values are represented
- * in a `UrlTree`. Since `UrlSerializer` already has a `parse` that takes a string, it already has control
- * over how a browser URL is parsed into a `UrlTree` on initial load/page refresh.
- */
-function normalizeQueryParams(k: string, v: unknown, urlSerializer: UrlSerializer): unknown {
-  const tree = new UrlTree();
-  tree.queryParams = {[k]: v};
-  return urlSerializer.parse(urlSerializer.serialize(tree)).queryParams[k];
-}
-
 function tree(
   oldRoot: UrlSegmentGroup,
   oldSegmentGroup: UrlSegmentGroup,
   newSegmentGroup: UrlSegmentGroup,
   queryParams: Params | null,
   fragment: string | null,
-  urlSerializer: UrlSerializer,
 ): UrlTree {
-  const qp: Params = {};
-  for (const [key, value] of Object.entries(queryParams ?? {})) {
-    // This retains old behavior where each item in the array was stringified individually This
-    // helps remove special-case handling for empty and single-item arrays where the default
-    // serializer removes empty arrays when serialized then parsed or converts them to non-arrays
-    // for single-item arrays. Changing this could have breaking change implications. Prior code
-    // always returned arrays of strings for array inputs so tests, applications, serializers,
-    // etc. may only be set up to handle string arrays. We could consider changing this in the
-    // future to serialize the entire array as a single value. For now, this feels safer and is
-    // at least a step in the right direction.
-    qp[key] = Array.isArray(value)
-      ? value.map((v) => normalizeQueryParams(key, v, urlSerializer))
-      : normalizeQueryParams(key, value, urlSerializer);
+  let qp: any = {};
+  if (queryParams) {
+    Object.entries(queryParams).forEach(([name, value]) => {
+      qp[name] = Array.isArray(value) ? value.map((v: any) => `${v}`) : `${value}`;
+    });
   }
 
   let rootCandidate: UrlSegmentGroup;

packages/router/src/recognize.ts

@@ -105,9 +105,9 @@ export class Recognizer {
       this.urlTree.queryParams,
       this.urlTree.fragment,
     );
+    // https://github.com/angular/angular/issues/47307
     // Creating the tree stringifies the query params
-    // We don't want to do this here to preserve pre-existing behavior
-    // so reassign them to the original.
+    // We don't want to do this here so reassign them to the original.
     tree.queryParams = this.urlTree.queryParams;
     routeState.url = this.urlSerializer.serialize(tree);
     return {state: routeState, tree};

packages/router/src/router.ts

@@ -495,13 +495,7 @@ export class Router {
       }
       relativeToUrlSegmentGroup = this.currentUrlTree.root;
     }
-    return createUrlTreeFromSegmentGroup(
-      relativeToUrlSegmentGroup,
-      commands,
-      q,
-      f ?? null,
-      this.urlSerializer,
-    );
+    return createUrlTreeFromSegmentGroup(relativeToUrlSegmentGroup, commands, q, f ?? null);
   }
 
   /**

packages/router/test/create_url_tree.spec.ts

@@ -16,7 +16,7 @@ import {Router} from '../src/router';
 import {RouterModule} from '../src/router_module';
 import {ActivatedRoute, ActivatedRouteSnapshot} from '../src/router_state';
 import {Params, PRIMARY_OUTLET} from '../src/shared';
-import {DefaultUrlSerializer, UrlSerializer, UrlTree} from '../src/url_tree';
+import {DefaultUrlSerializer, UrlTree} from '../src/url_tree';
 import {provideRouter, withRouterConfig} from '../src';
 import {timeout} from './helpers';
 
@@ -68,13 +68,6 @@ describe('createUrlTree', () => {
       expect(serializer.serialize(t2)).toEqual('/a/c/c2?n=1');
     });
 
-    it('should support parameter with single-item array', async () => {
-      await router.navigateByUrl('/a/c');
-      const t1 = create(router.routerState.root.children[0].children[0], ['c2'], {m: [1]});
-      expect(serializer.serialize(t1)).toEqual('/a/c/c2?m=1');
-      expect(t1.queryParams['m']).toEqual(['1']);
-    });
-
     it('should set query params', async () => {
       const p = serializer.parse('/');
       const t = await createRoot(p, [], {a: 'hey'});
@@ -88,17 +81,6 @@ describe('createUrlTree', () => {
       expect(t.queryParams).toEqual({a: '1'});
       expect(t.queryParamMap.get('a')).toEqual('1');
     });
-
-    it('should retain query parameter order', async () => {
-      await router.navigateByUrl('/a/c');
-      const t = create(router.routerState.root.children[0].children[0], ['c2'], {
-        z: 1,
-        a: '2',
-        m: [3, 4],
-        b: '5',
-      });
-      expect(serializer.serialize(t)).toEqual('/a/c/c2?z=1&a=2&m=3&m=4&b=5');
-    });
   });
 
   it('should navigate to the root', async () => {
@@ -764,78 +746,3 @@ async function advance(fixture: ComponentFixture<unknown>) {
   await timeout();
   fixture.detectChanges();
 }
-
-describe('createUrlTree with custom serializer', () => {
-  class CustomUrlSerializer extends DefaultUrlSerializer {
-    override parse(url: string): UrlTree {
-      const tree = super.parse(url);
-      const qp: {[key: string]: any} = {};
-      Object.keys(tree.queryParams).forEach((key) => {
-        const value = tree.queryParams[key];
-        if (typeof value === 'string' && (value.startsWith('{') || value.startsWith('['))) {
-          try {
-            qp[key] = JSON.parse(value);
-          } catch {}
-        } else {
-          qp[key] = value;
-        }
-      });
-      tree.queryParams = qp;
-      return tree;
-    }
-    override serialize(tree: UrlTree): string {
-      const qp: {[key: string]: any} = {};
-      Object.keys(tree.queryParams).forEach((key) => {
-        const value = tree.queryParams[key];
-        if (typeof value === 'object' && value !== null) {
-          qp[key] = JSON.stringify(value);
-        } else {
-          qp[key] = value;
-        }
-      });
-      tree.queryParams = qp;
-      return super.serialize(tree);
-    }
-  }
-
-  beforeEach(() => {
-    TestBed.configureTestingModule({
-      providers: [provideRouter([]), {provide: UrlSerializer, useClass: CustomUrlSerializer}],
-    });
-  });
-
-  it('should work with custom URL serializer that handles objects', async () => {
-    const router = TestBed.inject(Router);
-    const serializer = TestBed.inject(UrlSerializer);
-    const tree = router.createUrlTree(['a'], {queryParams: {someObject: {a: 1}}});
-    const url = serializer.serialize(tree);
-    expect(url).toEqual('/a?someObject=%7B%22a%22:1%7D');
-    const parsedTree = serializer.parse(url);
-    expect(parsedTree.queryParams).toEqual({someObject: {a: 1}});
-  });
-
-  it('should work with custom URL serializer and empty arrays', async () => {
-    const router = TestBed.inject(Router);
-    const serializer = TestBed.inject(UrlSerializer);
-    const tree = router.createUrlTree(['a'], {queryParams: {empty: []}});
-    const url = serializer.serialize(tree);
-    // with default serializer, 'empty' would be removed because serialization maps arrays to multiple
-    // key-value pairs, and an empty array would result in no key-value pairs
-    expect(url).toEqual('/a?empty=%5B%5D');
-    const parsedTree = serializer.parse(url);
-    expect(parsedTree.queryParams).toEqual({empty: []});
-  });
-
-  it('should work with custom URL serializer and single item arrays', async () => {
-    const router = TestBed.inject(Router);
-    const serializer = TestBed.inject(UrlSerializer);
-    const tree = router.createUrlTree(['a'], {queryParams: {one: ['1']}});
-    const url = serializer.serialize(tree);
-    expect(url).toEqual('/a?one=%5B%221%22%5D');
-    const parsedTree = serializer.parse(url);
-    // with default serializer, 'one' would be removed because serialization maps arrays to multiple
-    // key-value pairs, and a single item array would result in one key-value pair, which would be
-    // deserialized to a string, not an array
-    expect(parsedTree.queryParams).toEqual({one: ['1']});
-  });
-});