Skip to content

Commit 20fbd79

Browse files
sbarfurthsbarfurth
committed
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.
1 parent 9097b73 commit 20fbd79

File tree

5 files changed

+47
-17
lines changed

5 files changed

+47
-17
lines changed

‎adev/src/content/api-examples/common/pipes/ts/keyvalue_pipe.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {Component} from '@angular/core';
1717
<div *ngFor="let item of object | keyvalue">{{ item.key }}:{{ item.value }}</div>
1818
<p>Map</p>
1919
<div *ngFor="let item of map | keyvalue">{{ item.key }}:{{ item.value }}</div>
20+
<p>Natural order</p>
21+
<div *ngFor="let item of map | keyvalue: null">{{ item.key }}:{{ item.value }}</div>
2022
</span>
2123
`,
2224
standalone: false,

‎goldens/public-api/common/index.api.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -367,19 +367,19 @@ export interface KeyValue<K, V> {
367367
export class KeyValuePipe implements PipeTransform {
368368
constructor(differs: KeyValueDiffers);
369369
// (undocumented)
370-
transform<K, V>(input: ReadonlyMap<K, V>, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
370+
transform<K, V>(input: ReadonlyMap<K, V>, compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null): Array<KeyValue<K, V>>;
371371
// (undocumented)
372-
transform<K extends number, V>(input: Record<K, V>, compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number): Array<KeyValue<string, V>>;
372+
transform<K extends number, V>(input: Record<K, V>, compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null): Array<KeyValue<string, V>>;
373373
// (undocumented)
374-
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>>;
374+
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>>;
375375
// (undocumented)
376-
transform(input: null | undefined, compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number): null;
376+
transform(input: null | undefined, compareFn?: ((a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number) | null): null;
377377
// (undocumented)
378-
transform<K, V>(input: ReadonlyMap<K, V> | null | undefined, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>> | null;
378+
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;
379379
// (undocumented)
380-
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;
380+
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;
381381
// (undocumented)
382-
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;
382+
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;
383383
// (undocumented)
384384
static ɵfac: i0.ɵɵFactoryDeclaration<KeyValuePipe, never>;
385385
// (undocumented)

‎packages/common/src/pipes/keyvalue_pipe.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export interface KeyValue<K, V> {
3939
* The output array will be ordered by keys.
4040
* By default the comparator will be by Unicode point value.
4141
* You can optionally pass a compareFn if your keys are complex types.
42+
* Passing `null` as the compareFn will use natural ordering of the input.
4243
*
4344
* @usageNotes
4445
* ### Examples
@@ -60,7 +61,8 @@ export class KeyValuePipe implements PipeTransform {
6061

6162
private differ!: KeyValueDiffer<any, any>;
6263
private keyValues: Array<KeyValue<any, any>> = [];
63-
private compareFn: (a: KeyValue<any, any>, b: KeyValue<any, any>) => number = defaultComparator;
64+
private compareFn: ((a: KeyValue<any, any>, b: KeyValue<any, any>) => number) | null =
65+
defaultComparator;
6466

6567
/*
6668
* 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 {
6971
*/
7072
transform<K, V>(
7173
input: ReadonlyMap<K, V>,
72-
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
74+
compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
7375
): Array<KeyValue<K, V>>;
7476
transform<K extends number, V>(
7577
input: Record<K, V>,
76-
compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number,
78+
compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null,
7779
): Array<KeyValue<string, V>>;
7880
transform<K extends string, V>(
7981
input: Record<K, V> | ReadonlyMap<K, V>,
80-
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
82+
compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
8183
): Array<KeyValue<K, V>>;
8284
transform(
8385
input: null | undefined,
84-
compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number,
86+
compareFn?: ((a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number) | null,
8587
): null;
8688
transform<K, V>(
8789
input: ReadonlyMap<K, V> | null | undefined,
88-
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
90+
compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
8991
): Array<KeyValue<K, V>> | null;
9092
transform<K extends number, V>(
9193
input: Record<K, V> | null | undefined,
92-
compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number,
94+
compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null,
9395
): Array<KeyValue<string, V>> | null;
9496
transform<K extends string, V>(
9597
input: Record<K, V> | ReadonlyMap<K, V> | null | undefined,
96-
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number,
98+
compareFn?: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null,
9799
): Array<KeyValue<K, V>> | null;
98100
transform<K, V>(
99101
input: undefined | null | {[key: string]: V; [key: number]: V} | ReadonlyMap<K, V>,
100-
compareFn: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number = defaultComparator,
102+
compareFn: ((a: KeyValue<K, V>, b: KeyValue<K, V>) => number) | null = defaultComparator,
101103
): Array<KeyValue<K, V>> | null {
102104
if (!input || (!(input instanceof Map) && typeof input !== 'object')) {
103105
return null;
@@ -116,7 +118,9 @@ export class KeyValuePipe implements PipeTransform {
116118
});
117119
}
118120
if (differChanges || compareFnChanged) {
119-
this.keyValues.sort(compareFn);
121+
if (compareFn) {
122+
this.keyValues.sort(compareFn);
123+
}
120124
this.compareFn = compareFn;
121125
}
122126
return this.keyValues;

‎packages/common/test/pipes/keyvalue_pipe_spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ describe('KeyValuePipe', () => {
6161
{key: 'b', value: 1},
6262
]);
6363
});
64+
it('should not order by alpha when compareFn is null', () => {
65+
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
66+
expect(pipe.transform({'b': 1, 'a': 1}, null)).toEqual([
67+
{key: 'b', value: 1},
68+
{key: 'a', value: 1},
69+
]);
70+
});
6471
it('should reorder when compareFn changes', () => {
6572
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
6673
const input = {'b': 1, 'a': 2};
@@ -163,6 +170,21 @@ describe('KeyValuePipe', () => {
163170
{key: {id: 1}, value: 1},
164171
]);
165172
});
173+
it('should not order by alpha when compareFn is null', () => {
174+
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
175+
expect(
176+
pipe.transform(
177+
new Map([
178+
['b', 1],
179+
['a', 1],
180+
]),
181+
null,
182+
),
183+
).toEqual([
184+
{key: 'b', value: 1},
185+
{key: 'a', value: 1},
186+
]);
187+
});
166188
it('should reorder when compareFn changes', () => {
167189
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
168190
const input = new Map([

‎packages/examples/common/pipes/ts/keyvalue_pipe.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {Component} from '@angular/core';
1616
<div *ngFor="let item of object | keyvalue">{{ item.key }}:{{ item.value }}</div>
1717
<p>Map</p>
1818
<div *ngFor="let item of map | keyvalue">{{ item.key }}:{{ item.value }}</div>
19+
<p>Natural order</p>
20+
<div *ngFor="let item of map | keyvalue: null">{{ item.key }}:{{ item.value }}</div>
1921
</span>`,
2022
standalone: false,
2123
})

0 commit comments

Comments
 (0)