fix(common): correct and simplify typing of `KeyValuePipe` (#37447)

As shown in the tests, `KeyValuePipe.transform` can accept
`undefined`, in which case it always returns `null`.

Additionally, the typing for `string` keys can be made generic, so the
comparison function is only required to accept the relevant cases.

Finally, the typing for `number` records now shows that the comparison
function and the result entries will actually receive the string version
of the numeric keys, just as shown in the tests.

BREAKING CHANGE:
The typing of the `keyvalue` pipe has been fixed to report that for
input objects that have `number` keys, the result will contain the
string representation of the keys. This was already the case and the
code has simply been updated to reflect this. Please update the
consumers of the pipe output if they were relying on the incorrect
types. Note that this does not affect use cases where the input values
are `Map`s, so if you need to preserve `number`s, this is an effective
way.

PR Close #37447
This commit is contained in:
Andrea Canciani 2020-03-27 11:47:59 +01:00 committed by Alex Rickabaugh
parent 3b919ef10f
commit 4dfe0fa068
3 changed files with 34 additions and 37 deletions

View File

@ -140,21 +140,13 @@ export declare interface KeyValue<K, V> {
export declare class KeyValuePipe implements PipeTransform {
constructor(differs: KeyValueDiffers);
transform<K, V>(input: null, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): null;
transform<V>(input: {
[key: string]: V;
} | ReadonlyMap<string, V>, compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number): Array<KeyValue<string, V>>;
transform<V>(input: {
[key: string]: V;
} | ReadonlyMap<string, V> | null, compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number): Array<KeyValue<string, V>> | null;
transform<V>(input: {
[key: number]: V;
} | ReadonlyMap<number, V>, compareFn?: (a: KeyValue<number, V>, b: KeyValue<number, V>) => number): Array<KeyValue<number, V>>;
transform<V>(input: {
[key: number]: V;
} | ReadonlyMap<number, V> | null, compareFn?: (a: KeyValue<number, V>, b: KeyValue<number, V>) => number): Array<KeyValue<number, V>> | null;
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> | null, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>> | null;
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 string, V>(input: Record<K, V> | ReadonlyMap<K, V>, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
transform(input: null | undefined, compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number): null;
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 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 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;
}
export declare class Location {

View File

@ -50,31 +50,35 @@ export class KeyValuePipe implements PipeTransform {
private differ!: KeyValueDiffer<any, any>;
private keyValues: Array<KeyValue<any, any>> = [];
transform<K, V>(input: null, compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): null;
transform<V>(
input: {[key: string]: V}|ReadonlyMap<string, V>,
compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number):
Array<KeyValue<string, V>>;
transform<V>(
input: {[key: string]: V}|ReadonlyMap<string, V>|null,
compareFn?: (a: KeyValue<string, V>, b: KeyValue<string, V>) => number):
Array<KeyValue<string, V>>|null;
transform<V>(
input: {[key: number]: V}|ReadonlyMap<number, V>,
compareFn?: (a: KeyValue<number, V>, b: KeyValue<number, V>) => number):
Array<KeyValue<number, V>>;
transform<V>(
input: {[key: number]: V}|ReadonlyMap<number, V>|null,
compareFn?: (a: KeyValue<number, V>, b: KeyValue<number, V>) => number):
Array<KeyValue<number, V>>|null;
/*
* NOTE: when the `input` value is a simple Record<K, V> object, the keys are extracted with
* Object.keys(). This means that even if the `input` type is Record<number, V> the keys are
* compared/returned as `string`s.
*/
transform<K, V>(
input: ReadonlyMap<K, V>,
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
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 string, V>(
input: Record<K, V>|ReadonlyMap<K, V>,
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): Array<KeyValue<K, V>>;
transform(
input: null|undefined,
compareFn?: (a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number): null;
transform<K, V>(
input: ReadonlyMap<K, V>|null,
input: ReadonlyMap<K, V>|null|undefined,
compareFn?: (a: KeyValue<K, V>, b: KeyValue<K, V>) => number): 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):
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): Array<KeyValue<K, V>>|null;
transform<K, V>(
input: null|{[key: string]: V, [key: number]: V}|ReadonlyMap<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):
Array<KeyValue<K, V>>|null {
if (!input || (!(input instanceof Map) && typeof input !== 'object')) {

View File

@ -17,12 +17,12 @@ describe('KeyValuePipe', () => {
});
it('should return null when given undefined', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
expect(pipe.transform(undefined as any)).toEqual(null);
expect(pipe.transform(undefined)).toEqual(null);
});
it('should return null for an unsupported type', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const fn = () => {};
expect(pipe.transform(fn as any)).toEqual(null);
expect(pipe.transform(fn as any as null)).toEqual(null);
});
describe('object dictionary', () => {
it('should return empty array of an empty dictionary', () => {
@ -98,8 +98,9 @@ describe('KeyValuePipe', () => {
});
it('should order by numerical and alpha', () => {
const pipe = new KeyValuePipe(defaultKeyValueDiffers);
const input = [[2, 1], [1, 1], ['b', 1], [0, 1], [3, 1], ['a', 1]];
expect(pipe.transform(new Map(input as any))).toEqual([
const input =
[[2, 1], [1, 1], ['b', 1], [0, 1], [3, 1], ['a', 1]] as Array<[number | string, number]>;
expect(pipe.transform(new Map(input))).toEqual([
{key: 0, value: 1}, {key: 1, value: 1}, {key: 2, value: 1}, {key: 3, value: 1},
{key: 'a', value: 1}, {key: 'b', value: 1}
]);