mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
fix(common): preserve literal key union in KeyValuePipe.transform()
Previously, when you passed an object typed like
Record<'a' | 'b', number> into the `keyvalue` pipe, TypeScript would
"forget" that the keys could only ever be 'a' or 'b', and just tell
you the key was a plain `string` instead. So code like this used to
fail to compile, even though it's correct:
```ts
const input: Record<'a' | 'b', number> = {a: 1, b: 2};
const result = pipe.transform(input);
const key: 'a' | 'b' = result[0].key; // error: string is not 'a' | 'b'
```
This happened because the pipe has multiple overloaded versions of
transform(), and TypeScript checks them top to bottom, using the
first one that matches. The "number keys" overload was listed first,
and it happened to also match string-keyed objects by accident, so
it "won" before the correct "string keys" overload ever got a
chance to run.
The fix just reorders those two overloads so the string-keys one is
checked first. Nothing about runtime behavior changes — objects with
actual numeric keys (e.g. Record<1 | 2, string>) still correctly
report their keys as plain `string`, matching what Object.keys()
really returns at runtime.
This commit is contained in:
@@ -379,18 +379,18 @@ export class KeyValuePipe implements PipeTransform {
|
||||
// (undocumented)
|
||||
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) | 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) | 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) | null): Array<KeyValue<string, V>>;
|
||||
// (undocumented)
|
||||
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) | 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) | 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) | 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) | null): Array<KeyValue<string, V>> | null;
|
||||
// (undocumented)
|
||||
transform<T>(input: T, compareFn?: T extends object ? (a: T[keyof T], b: T[keyof T]) => number : never): T extends object ? Array<KeyValue<keyof T, T[keyof T]>> : null;
|
||||
// (undocumented)
|
||||
static ɵfac: i0.ɵɵFactoryDeclaration<KeyValuePipe, never>;
|
||||
|
||||
@@ -75,14 +75,23 @@ export class KeyValuePipe implements PipeTransform {
|
||||
input: ReadonlyMap<K, V>,
|
||||
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) | null,
|
||||
): Array<KeyValue<string, V>>;
|
||||
// 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<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>>;
|
||||
transform<K extends number, V>(
|
||||
input: Record<K, V>,
|
||||
compareFn?: ((a: KeyValue<string, V>, b: KeyValue<string, V>) => number) | null,
|
||||
): Array<KeyValue<string, V>>;
|
||||
transform(
|
||||
input: null | undefined,
|
||||
compareFn?: ((a: KeyValue<unknown, unknown>, b: KeyValue<unknown, unknown>) => number) | null,
|
||||
@@ -91,16 +100,16 @@ export class KeyValuePipe implements PipeTransform {
|
||||
input: ReadonlyMap<K, V> | null | undefined,
|
||||
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) | 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) | 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) | null,
|
||||
): Array<KeyValue<string, V>> | null;
|
||||
|
||||
transform<T>(
|
||||
input: T,
|
||||
compareFn?: T extends object ? (a: T[keyof T], b: T[keyof T]) => number : never,
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user