feat(common): disable keyvalue sorting using null compareFn

The keyvalue pipe sorts the entries of the input by key. This has been the subject of debate in the past (#42490). The core of the discussions is that it is often desirable (and perhaps expected) that they natural ordering of the input is respected. There are at least two workarounds to restore natural ordering, such as a `compareFn` that simply returns `1` or a custom pipe. However, both of these require code for pipe consumers to maintain or copy around to many places. Allowing `null` as `compareFn` and treating it as "natural order" is fairly simple to understand, backward compatible and was suggested a few times on #42490 where it seemed to be received well. Using `null` is also possible in templates without any component code changes.
angular · sbarfurth · Aug 22, 2024 · Oct 16, 2024 · 20fbd79460c36f2d48bd71dd53413dffd76b9749
commit 20fbd79460c36f2d48bd71dd53413dffd76b9749
diff --git a/adev/src/content/api-examples/common/pipes/ts/keyvalue_pipe.ts b/adev/src/content/api-examples/common/pipes/ts/keyvalue_pipe.ts
@@ -17,6 +17,8 @@ import {Component} from '@angular/core';
       <div *ngFor="let item of object | keyvalue">{{ item.key }}:{{ item.value }}</div>
       <p>Map</p>
       <div *ngFor="let item of map | keyvalue">{{ item.key }}:{{ item.value }}</div>
+      <p>Natural order</p>
+      <div *ngFor="let item of map | keyvalue: null">{{ item.key }}:{{ item.value }}</div>
     </span>
   `,
   standalone: false,

diff --git a/goldens/public-api/common/index.api.md b/goldens/public-api/common/index.api.md
@@ -367,19 +367,19 @@ export interface KeyValue<K, V> {
 export class KeyValuePipe implements PipeTransform {
     constructor(differs: KeyValueDiffers);
     // (undocumented)
-    transform<K, V>(input: ReadonlyMap<K, V>, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
+    transform<K, V>(input: ReadonlyMap<K, V>, compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null): Array<KeyValue<K, V>>;
     // (undocumented)
-    transform<K extends number, V>(input: Record<K, V>, compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number): Array<KeyValue<string, V>>;
+    transform<K extends number, V>(input: Record<K, V>, compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null): Array<KeyValue<string, V>>;
     // (undocumented)
-    transform<K extends string, V>(input: Record<K, V> | ReadonlyMap<K, V>, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
+    transform<K extends string, V>(input: Record<K, V> | ReadonlyMap<K, V>, compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null): Array<KeyValue<K, V>>;
     // (undocumented)
-    transform(input: null | undefined, compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number): null;
+    transform(input: null | undefined, compareFn?: ((a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number) | null): null;
     // (undocumented)
-    transform<K, V>(input: ReadonlyMap<K, V> | null | undefined, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>> | null;
+    transform<K, V>(input: ReadonlyMap<K, V> | null | undefined, compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null): Array<KeyValue<K, V>> | null;
     // (undocumented)
-    transform<K extends number, V>(input: Record<K, V> | null | undefined, compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number): Array<KeyValue<string, V>> | null;
+    transform<K extends number, V>(input: Record<K, V> | null | undefined, compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null): Array<KeyValue<string, V>> | null;
     // (undocumented)
-    transform<K extends string, V>(input: Record<K, V> | ReadonlyMap<K, V> | null | undefined, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>> | null;
+    transform<K extends string, V>(input: Record<K, V> | ReadonlyMap<K, V> | null | undefined, compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null): Array<KeyValue<K, V>> | null;
     // (undocumented)
     static ɵfac: i0.ɵɵFactoryDeclaration<KeyValuePipe, never>;
     // (undocumented)

diff --git a/packages/common/src/pipes/keyvalue_pipe.ts b/packages/common/src/pipes/keyvalue_pipe.ts
@@ -39,6 +39,7 @@ export interface KeyValue<K, V> {
  * The output array will be ordered by keys.
  * By default the comparator will be by Unicode point value.
  * You can optionally pass a compareFn if your keys are complex types.
+ * Passing `null` as the compareFn will use natural ordering of the input.
  *
  * @usageNotes
  * ### Examples
@@ -60,7 +61,8 @@ export class KeyValuePipe implements PipeTransform {
 
   private differ!: KeyValueDiffer<any, any>;
   private keyValues: Array<KeyValue<any, any>> = [];
-  private compareFn: (a: KeyValue<any, any>, b: KeyValue<any, any>) => number = defaultComparator;
+  private compareFn: ((a: KeyValue<any, any>, b: KeyValue<any, any>) => number) | null =
+    defaultComparator;
 
   /*
    * NOTE: when the `input` value is a simple Record<K, V> object, the keys are extracted with
@@ -69,35 +71,35 @@ export class KeyValuePipe implements PipeTransform {
    */
   transform<K, V>(
     input: ReadonlyMap<K, V>,
-    compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
+    compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
   ): Array<KeyValue<K, V>>;
   transform<K extends number, V>(
     input: Record<K, V>,
-    compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number,
+    compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null,
   ): Array<KeyValue<string, V>>;
   transform<K extends string, V>(
     input: Record<K, V> | ReadonlyMap<K, V>,
-    compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
+    compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
   ): Array<KeyValue<K, V>>;
   transform(
     input: null | undefined,
-    compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number,
+    compareFn?: ((a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number) | null,
   ): null;
   transform<K, V>(
     input: ReadonlyMap<K, V> | null | undefined,
-    compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
+    compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
   ): Array<KeyValue<K, V>> | null;
   transform<K extends number, V>(
     input: Record<K, V> | null | undefined,
-    compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number,
+    compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null,
   ): Array<KeyValue<string, V>> | null;
   transform<K extends string, V>(
     input: Record<K, V> | ReadonlyMap<K, V> | null | undefined,
-    compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
+    compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
   ): Array<KeyValue<K, V>> | null;
   transform<K, V>(
     input: undefined | null | {[key: string]: V; [key: number]: V} | ReadonlyMap<K, V>,
-    compareFn: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number = defaultComparator,
+    compareFn: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null = defaultComparator,
   ): Array<KeyValue<K, V>> | null {
     if (!input || (!(input instanceof Map) && typeof input !== 'object')) {
       return null;
@@ -116,7 +118,9 @@ export class KeyValuePipe implements PipeTransform {
       });
     }
     if (differChanges || compareFnChanged) {
-      this.keyValues.sort(compareFn);
+      if (compareFn) {
+        this.keyValues.sort(compareFn);
+      }
       this.compareFn = compareFn;
     }
     return this.keyValues;

diff --git a/packages/common/test/pipes/keyvalue_pipe_spec.ts b/packages/common/test/pipes/keyvalue_pipe_spec.ts
@@ -61,6 +61,13 @@ describe('KeyValuePipe', () => {
         {key: 'b', value: 1},
       ]);
     });
+    it('should not order by alpha when compareFn is null', () => {
+      const pipe = new KeyValuePipe(defaultKeyValueDiffers);
+      expect(pipe.transform({'b': 1, 'a': 1}, null)).toEqual([
+        {key: 'b', value: 1},
+        {key: 'a', value: 1},
+      ]);
+    });
     it('should reorder when compareFn changes', () => {
       const pipe = new KeyValuePipe(defaultKeyValueDiffers);
       const input = {'b': 1, 'a': 2};
@@ -163,6 +170,21 @@ describe('KeyValuePipe', () => {
         {key: {id: 1}, value: 1},
       ]);
     });
+    it('should not order by alpha when compareFn is null', () => {
+      const pipe = new KeyValuePipe(defaultKeyValueDiffers);
+      expect(
+        pipe.transform(
+          new Map([
+            ['b', 1],
+            ['a', 1],
+          ]),
+          null,
+        ),
+      ).toEqual([
+        {key: 'b', value: 1},
+        {key: 'a', value: 1},
+      ]);
+    });
     it('should reorder when compareFn changes', () => {
       const pipe = new KeyValuePipe(defaultKeyValueDiffers);
       const input = new Map([

diff --git a/packages/examples/common/pipes/ts/keyvalue_pipe.ts b/packages/examples/common/pipes/ts/keyvalue_pipe.ts
@@ -16,6 +16,8 @@ import {Component} from '@angular/core';
     <div *ngFor="let item of object | keyvalue">{{ item.key }}:{{ item.value }}</div>
     <p>Map</p>
     <div *ngFor="let item of map | keyvalue">{{ item.key }}:{{ item.value }}</div>
+    <p>Natural order</p>
+    <div *ngFor="let item of map | keyvalue: null">{{ item.key }}:{{ item.value }}</div>
   </span>`,
   standalone: false,
 })