Skip to content

Commit

Permalink
fix(common): re-sort output of KeyValuePipe when compareFn changes (
Browse files Browse the repository at this point in the history
#42821)

Previously, if only the `compareFn` changed but the data itself did not, then
the `KeyValuePipe` did not re-sort the output.

Fixes #42819

PR Close #42821
  • Loading branch information
meblum authored and AndrewKushnir committed Jul 13, 2021
1 parent b101ad0 commit d654c79
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions packages/common/src/pipes/keyvalue_pipe.ts
Expand Up @@ -49,6 +49,7 @@ 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;

/*
* NOTE: when the `input` value is a simple Record<K, V> object, the keys are extracted with
Expand Down Expand Up @@ -91,13 +92,17 @@ export class KeyValuePipe implements PipeTransform {
}

const differChanges: KeyValueChanges<K, V>|null = this.differ.diff(input as any);
const compareFnChanged = compareFn !== this.compareFn;

if (differChanges) {
this.keyValues = [];
differChanges.forEachItem((r: KeyValueChangeRecord<K, V>) => {
this.keyValues.push(makeKeyValuePair(r.key, r.currentValue!));
});
}
if (differChanges || compareFnChanged) {
this.keyValues.sort(compareFn);
this.compareFn = compareFn;
}
return this.keyValues;
}
Expand Down
18 changes: 18 additions & 0 deletions packages/common/test/pipes/keyvalue_pipe_spec.ts
Expand Up @@ -51,6 +51,15 @@ describe('KeyValuePipe', () => {
{key: 'a', value: 1}, {key: 'b', value: 1}
]);
});
it('should reorder when compareFn changes', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const input = {'b': 1, 'a': 2};
pipe.transform<string, number>(input);
expect(pipe.transform<string, number>(input, (a, b) => a.value - b.value)).toEqual([
{key: 'b', value: 1},
{key: 'a', value: 2},
]);
});
it('should return the same ref if nothing changes', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const transform1 = pipe.transform({1: 2});
Expand Down Expand Up @@ -114,6 +123,15 @@ describe('KeyValuePipe', () => {
{key: {id: 1}, value: 1},
]);
});
it('should reorder when compareFn changes', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const input = new Map([['b', 1], ['a', 2]]);
pipe.transform<string, number>(input);
expect(pipe.transform<string, number>(input, (a, b) => a.value - b.value)).toEqual([
{key: 'b', value: 1},
{key: 'a', value: 2},
]);
});
it('should return the same ref if nothing changes', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const transform1 = pipe.transform(new Map([[1, 2]]));
Expand Down

0 comments on commit d654c79

Please sign in to comment.