diff --git a/goldens/public-api/common/index.api.md b/goldens/public-api/common/index.api.md index 211fcfa68334..4a5a43b7a270 100644 --- a/goldens/public-api/common/index.api.md +++ b/goldens/public-api/common/index.api.md @@ -379,18 +379,18 @@ export class KeyValuePipe implements PipeTransform { // (undocumented) transform(input: ReadonlyMap, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array>; // (undocumented) - transform(input: Record, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array>; - // (undocumented) transform(input: Record | ReadonlyMap, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array>; // (undocumented) + transform(input: Record, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array>; + // (undocumented) transform(input: null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): null; // (undocumented) transform(input: ReadonlyMap | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array> | null; // (undocumented) - transform(input: Record | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array> | null; - // (undocumented) transform(input: Record | ReadonlyMap | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array> | null; // (undocumented) + transform(input: Record | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null): Array> | null; + // (undocumented) transform(input: T, compareFn?: T extends object ? (a: T[keyof T], b: T[keyof T]) => number : never): T extends object ? Array> : null; // (undocumented) static ɵfac: i0.ɵɵFactoryDeclaration; diff --git a/packages/common/src/pipes/keyvalue_pipe.ts b/packages/common/src/pipes/keyvalue_pipe.ts index 17e61214d401..8278f432075f 100644 --- a/packages/common/src/pipes/keyvalue_pipe.ts +++ b/packages/common/src/pipes/keyvalue_pipe.ts @@ -75,14 +75,23 @@ export class KeyValuePipe implements PipeTransform { input: ReadonlyMap, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, ): Array>; - transform( - input: Record, - compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, - ): Array>; + // TypeScript tries overloads top-to-bottom and stops at the first match. A + // plain object with string-literal keys (e.g. `Record<'a' | 'b', V>`) also + // happens to satisfy the more general `K extends number` overload below once + // inference falls back to its constraint — so if that overload is checked + // first, it wins by accident and silently widens `key` from `'a' | 'b'` to + // plain `string`. Putting the string overload first means it claims + // string-keyed input before the number overload ever gets a chance, while + // genuinely numeric-keyed input (which fails `K extends string`) still falls + // through to the number overload exactly as before. transform( input: Record | ReadonlyMap, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, ): Array>; + transform( + input: Record, + compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, + ): Array>; transform( input: null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, @@ -91,16 +100,16 @@ export class KeyValuePipe implements PipeTransform { input: ReadonlyMap | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, ): Array> | null; - transform( - input: Record | null | undefined, - compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, - ): Array> | null; - transform( input: Record | ReadonlyMap | null | undefined, compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, ): Array> | null; + transform( + input: Record | null | undefined, + compareFn?: ((a: KeyValue, b: KeyValue) => number) | null, + ): Array> | null; + transform( input: T, compareFn?: T extends object ? (a: T[keyof T], b: T[keyof T]) => number : never, diff --git a/packages/common/test/pipes/keyvalue_pipe_spec.ts b/packages/common/test/pipes/keyvalue_pipe_spec.ts index 9b4727f5eef5..d63e39a82921 100644 --- a/packages/common/test/pipes/keyvalue_pipe_spec.ts +++ b/packages/common/test/pipes/keyvalue_pipe_spec.ts @@ -100,6 +100,38 @@ describe('KeyValuePipe', () => { expect(pipe.transform(value)).toEqual(null); }); + it('should preserve a literal key union instead of widening it to string', () => { + const pipe = new KeyValuePipe(defaultKeyValueDiffers); + const input: Record<'a' | 'b', number> = {a: 1, b: 2}; + const result = pipe.transform(input); + + // Compile-time check: if `key` had widened back to plain `string`, this + // assignment would fail to compile — that's the actual bug from #43883. + const key: 'a' | 'b' = result[0].key; + + expect(key).toBe('a'); + expect(result).toEqual([ + {key: 'a', value: 1}, + {key: 'b', value: 2}, + ]); + }); + + it('should still collapse a numerically-keyed object to string keys', () => { + const pipe = new KeyValuePipe(defaultKeyValueDiffers); + const input: Record<1 | 2, string> = {1: 'one', 2: 'two'}; + const result = pipe.transform(input); + + // Numeric keys become strings at runtime (Object.keys() always returns + // strings), so `key` should stay `string`, not narrow to `1 | 2`. + const key: string = result[0].key; + + expect(key).toBe('1'); + expect(result).toEqual([ + {key: '1', value: 'one'}, + {key: '2', value: 'two'}, + ]); + }); + it('should accept an object with optional keys', () => { interface MyInterface { one: string;