Skip to content

Commit

Permalink
Changing UpdateData<T> to expand support for types with index signatu…
Browse files Browse the repository at this point in the history
…res. (#7318)

Changing UpdateData<T> to expand support for types with index signatures.
  • Loading branch information
MarkDuckworth authored Jun 30, 2023
1 parent f2fb56f commit 57f2a86
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/breezy-flies-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": minor
---

Changing UpdateData<T> to expand support for types with index signatures.
2 changes: 1 addition & 1 deletion common/api-review/firestore-lite.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function addDoc<AppModelType, DbModelType extends DocumentData>(reference

// @public
export type AddPrefixToKeys<Prefix extends string, T extends Record<string, unknown>> = {
[K in keyof T & string as `${Prefix}.${K}`]+?: T[K];
[K in keyof T & string as `${Prefix}.${K}`]+?: string extends K ? any : T[K];
};

// @public
Expand Down
2 changes: 1 addition & 1 deletion common/api-review/firestore.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function addDoc<AppModelType, DbModelType extends DocumentData>(reference

// @public
export type AddPrefixToKeys<Prefix extends string, T extends Record<string, unknown>> = {
[K in keyof T & string as `${Prefix}.${K}`]+?: T[K];
[K in keyof T & string as `${Prefix}.${K}`]+?: string extends K ? any : T[K];
};

// @public
Expand Down
2 changes: 1 addition & 1 deletion docs-devsite/firestore_.md
Original file line number Diff line number Diff line change
Expand Up @@ -2209,7 +2209,7 @@ Returns a new map where every key is prefixed with the outer key appended to a d

```typescript
export declare type AddPrefixToKeys<Prefix extends string, T extends Record<string, unknown>> = {
[K in keyof T & string as `${Prefix}.${K}`]+?: T[K];
[K in keyof T & string as `${Prefix}.${K}`]+?: string extends K ? any : T[K];
};
```

Expand Down
2 changes: 1 addition & 1 deletion docs-devsite/firestore_lite.md
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,7 @@ Returns a new map where every key is prefixed with the outer key appended to a d

```typescript
export declare type AddPrefixToKeys<Prefix extends string, T extends Record<string, unknown>> = {
[K in keyof T & string as `${Prefix}.${K}`]+?: T[K];
[K in keyof T & string as `${Prefix}.${K}`]+?: string extends K ? any : T[K];
};
```

Expand Down
17 changes: 16 additions & 1 deletion packages/firestore/src/lite-api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,22 @@ export type AddPrefixToKeys<
T extends Record<string, unknown>
> =
// Remap K => Prefix.K. See https://www.typescriptlang.org/docs/handbook/2/mapped-types.html#key-remapping-via-as
{ [K in keyof T & string as `${Prefix}.${K}`]+?: T[K] };

// `string extends K : ...` is used to detect index signatures
// like `{[key: string]: bool}`. We map these properties to type `any`
// because a field path like `foo.[string]` will match `foo.bar` or a
// sub-path `foo.bar.baz`. Because it matches a sub-path, we have to
// make this type `any` to allow for any types of the sub-path property.
// This is a significant downside to using index signatures in types for `T`
// for `UpdateData<T>`.

{
/* eslint-disable @typescript-eslint/no-explicit-any */
[K in keyof T & string as `${Prefix}.${K}`]+?: string extends K
? any
: T[K];
/* eslint-enable @typescript-eslint/no-explicit-any */
};

/**
* Given a union type `U = T1 | T2 | ...`, returns an intersected type
Expand Down
104 changes: 91 additions & 13 deletions packages/firestore/test/unit/lite-api/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,23 +441,101 @@ describe('UpdateData - v9', () => {
}
};

// preserves type - failure
_ = {
// @ts-expect-error
'indexed.bar': false,
// @ts-expect-error
'indexed.baz': 'string'
expect(true).to.be.true;
});
});

// v10 tests cover new scenarios that are fixed for v10
describe('UpdateData - v10', () => {
interface MyV10ServerType {
booleanProperty: boolean;

// index signatures nested 1 layer deep
indexed: {
[name: string]: {
booleanProperty: boolean;
numberProperty: number;
};
};

// preserves properties of nested objects - failure
_ = {
'indexed.bar': {
// @ts-expect-error
booleanProperty: 'string'
}
// index signatures nested 2 layers deep
layer: {
indexed: {
[name: string]: {
booleanProperty: boolean;
numberProperty: number;
};
};
};
}

expect(true).to.be.true;
describe('given nested objects with index properties', () => {
it('supports object replacement at each layer (with partial)', () => {
// This unexpectidly fails in v9 when the object has index signature nested
// two layers deep (e.g. layer.indexed.[name]).
const _: UpdateData<MyV10ServerType> = {
indexed: {
bar: {},
baz: {}
}
};

expect(true).to.be.true;
});

it('allows dot notation for nested index types', () => {
let _: UpdateData<MyV10ServerType>;

// v10 allows 3 layers of dot notation

// allows the property
_ = {
'indexed.bar.booleanProperty': true
};

_ = {
'indexed.bar.numberProperty': 1
};

// does not enforce type
_ = {
'indexed.bar.booleanProperty': 'string value is not rejected'
};

_ = {
'indexed.bar.numberProperty': 'string value is not rejected'
};

// rejects properties that don't exist
_ = {
'indexed.bar.unknown': 'string value is not rejected'
};

expect(true).to.be.true;
});

it('allows dot notation for nested index types that are 2 layers deep', () => {
let _: UpdateData<MyV10ServerType>;

// v10 3 layers with dot notation

// allows the property
_ = {
'layer.indexed.bar.booleanProperty': true
};

// allows the property, but does not enforce type
_ = {
'layer.indexed.bar.booleanProperty': 'string value is not rejected'
};

// Allows unknown properties in sub types
_ = {
'layer.indexed.bar.unknownProperty': 'This just allows anything'
};

expect(true).to.be.true;
});
});
});
});
Expand Down

0 comments on commit 57f2a86

Please sign in to comment.