From a9b78e356996fa57f093363dfedffa0a81e37bbc Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Mon, 18 Dec 2023 16:07:14 -0700 Subject: [PATCH 1/4] Fix: UpdateData to allow indexed types or Record for T. --- packages/firestore/src/lite-api/reference.ts | 12 +- packages/firestore/src/lite-api/types.ts | 23 +- .../test/unit/lite-api/types.test.ts | 320 ++++++++++++++++++ 3 files changed, 351 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/lite-api/reference.ts b/packages/firestore/src/lite-api/reference.ts index 89836065c99..3ff27dcbf6a 100644 --- a/packages/firestore/src/lite-api/reference.ts +++ b/packages/firestore/src/lite-api/reference.ts @@ -38,7 +38,7 @@ import { Firestore } from './database'; import { FieldPath } from './field_path'; import { FieldValue } from './field_value'; import { FirestoreDataConverter } from './snapshot'; -import { NestedUpdateFields, Primitive } from './types'; +import { ChildTypes, NestedUpdateFields, Primitive } from './types'; /** * Document data (for use with {@link @firebase/firestore/lite#(setDoc:1)}) consists of fields mapped to @@ -83,8 +83,16 @@ export type WithFieldValue = export type UpdateData = T extends Primitive ? T : T extends {} - ? { [K in keyof T]?: UpdateData | FieldValue } & NestedUpdateFields + ? { + // If `string extends K`, this is an index signature like + // `{[key: string]: { foo: bool }}`. In the generated UpdateData + // indexed properties can match their type or any child types. + [K in keyof T]?: string extends K + ? PartialWithFieldValue> + : UpdateData | FieldValue; + } & NestedUpdateFields : Partial; + /** * An options object that configures the behavior of {@link @firebase/firestore/lite#(setDoc:1)}, {@link * @firebase/firestore/lite#(WriteBatch.set:1)} and {@link @firebase/firestore/lite#(Transaction.set:1)} calls. These calls can be diff --git a/packages/firestore/src/lite-api/types.ts b/packages/firestore/src/lite-api/types.ts index 3c697e63b41..be0246e82bd 100644 --- a/packages/firestore/src/lite-api/types.ts +++ b/packages/firestore/src/lite-api/types.ts @@ -34,7 +34,13 @@ export type Primitive = string | number | boolean | undefined | null; export type NestedUpdateFields> = UnionToIntersection< { - [K in keyof T & string]: ChildUpdateFields; + // If `string extends K`, this is an index signature like + // `{[key: string]: { foo: bool }}`. We map these properties to + // `never`, which prevents prefixing a nested key with `[string]`. + // We don't want to generate a field like `[string].foo: bool`. + [K in keyof T & string]: string extends K + ? never + : ChildUpdateFields; }[keyof T & string] // Also include the generated prefix-string keys. >; @@ -57,6 +63,18 @@ export type ChildUpdateFields = : // UpdateData is always a map of values. never; +/** + * For the given type, return a union type of T + * and the types of all child properties of T. + */ +export type ChildTypes = T extends Record + ? + | { + [K in keyof T & string]: ChildTypes; + }[keyof T & string] + | T + : T; + /** * Returns a new map where every key is prefixed with the outer key appended * to a dot. @@ -78,7 +96,8 @@ export type AddPrefixToKeys< { /* eslint-disable @typescript-eslint/no-explicit-any */ [K in keyof T & string as `${Prefix}.${K}`]+?: string extends K - ? any + ? // TODO(b/316955294): Replace `any` with `ChildTypes` (breaking change). + any : T[K]; /* eslint-enable @typescript-eslint/no-explicit-any */ }; diff --git a/packages/firestore/test/unit/lite-api/types.test.ts b/packages/firestore/test/unit/lite-api/types.test.ts index 55232aeee60..2d01b8fdb8e 100644 --- a/packages/firestore/test/unit/lite-api/types.test.ts +++ b/packages/firestore/test/unit/lite-api/types.test.ts @@ -16,6 +16,7 @@ */ import { expect } from 'chai'; +import { Firestore, runTransaction } from '../../../lite/index'; import { DocumentData, DocumentReference, @@ -45,6 +46,10 @@ interface MyObjectType { nullProperty: null; undefinedProperty: undefined; unionProperty: MyUnionType; + objectProperty: { + booleanProperty: boolean; + stringProperty: string; + }; } // v9 tests cover scenarios that worked in @@ -538,6 +543,321 @@ describe('UpdateData - v9', () => { }); }); }); + + describe('given Record', () => { + it('supports primitive type for T', () => { + let _: UpdateData>; + + _ = { + numberProperty: 1 + }; + + _ = { + // @ts-expect-error Unsupported type + numberProperty: false + }; + + expect(true).to.be.true; + }); + + it('supports object type for T', () => { + let _: UpdateData>>; + + _ = {}; + + _ = { + indexedProperty: {} + }; + + _ = { + indexedProperty: { + numberProperty: 1, + booleanProperty: true + } + }; + + _ = { + indexedProperty: { + objectProperty: {} + } + }; + + _ = { + indexedProperty: { + objectProperty: { + booleanProperty: true + } + } + }; + + _ = { + indexedProperty: { + stringProperty: 'string' + } + }; + + _ = { + indexedProperty: { + numberProperty: 1, + booleanProperty: true, + stringProperty: 'string', + // @ts-expect-error Unsupported type + nullProperty: null, + undefinedProperty: undefined, + unionProperty: 1, + objectProperty: { + stringProperty: 'string', + booleanProperty: true + } + } + }; + + // It allows any child property type + // when the property is indexed. + _ = { + indexedProperty: false + }; + + // It allows any child property type + // when the property is indexed. + _ = { + indexedProperty: 'string' + }; + + // It prevents types that are not a + // child property type. + _ = { + // @ts-expect-error Unsupported type + indexedProperty: null + }; + + // It allows dot notation to set nested properties + _ = { + 'indexedProperty.stringProperty': 'string' + }; + + // It allows dot notation to set nested properties, + // but only enforces types to any of the child properties + // of the indexed property. + _ = { + 'indexedProperty.stringProperty': true, + 'indexedProperty.booleanProperty': 'string', + // @ts-expect-error Unsupported type + 'indexedProperty.undefinedProperty': null + }; + + // But still enforces property types + // when the child type is object + _ = { + objectProperty: { + // @ts-expect-error Unsupported type + numberProperty: false + } + }; + + _ = { + objectProperty: { + // @ts-expect-error Unsupported type + unknownProperty: false + } + }; + + expect(true).to.be.true; + }); + + it('supports object with nested index for T', () => { + let _: UpdateData< + Record< + string, + { + objectWithIndexProperty: { + [key: string]: boolean; + }; + deepObjectWithIndexProperty: { + [key: string]: { + stringProperty: string; + numberProperty: number; + }; + }; + } + > + >; + + _ = {}; + + _ = { + indexedProperty: {} + }; + + _ = { + indexedProperty: { + objectWithIndexProperty: {}, + deepObjectWithIndexProperty: {} + } + }; + + _ = { + indexedProperty: { + objectWithIndexProperty: {} + } + }; + + _ = { + indexedProperty: { + objectWithIndexProperty: { + indexedProperty: true + } + } + }; + + _ = { + indexedProperty: { + deepObjectWithIndexProperty: { + indexedProperty: {} + } + } + }; + + _ = { + indexedProperty: { + deepObjectWithIndexProperty: { + indexedProperty: { + stringProperty: 'string' + } + } + } + }; + + _ = { + indexedProperty: { + stringProperty: 'string' + } + }; + + _ = { + indexedProperty: { + objectWithIndexProperty: { + indexedProperty: true + }, + deepObjectWithIndexProperty: { + indexedProperty: { + stringProperty: 'string', + numberProperty: 1 + } + } + } + }; + + // It allows any child property type + // when the property is indexed. + _ = { + indexedProperty: false + }; + + // It allows any child property type + // when the property is indexed. + _ = { + indexedProperty: 'string' + }; + + // It prevents types that are not a + // child property type. + _ = { + // @ts-expect-error Unsupported type + indexedProperty: null + }; + + // It allows dot notation to set nested properties + _ = { + 'indexedProperty.stringProperty': 'string' + }; + + // It allows dot notation to set nested properties, + // but only enforces types to any of the child properties + // of the indexed property. + _ = { + 'indexedProperty.stringProperty': true, + 'indexedProperty.booleanProperty': 'string', + // @ts-expect-error Unsupported type + 'indexedProperty.undefinedProperty': null + }; + + // But still enforces property types + // when the child type is object + _ = { + indexedProperty: { + // @ts-expect-error Unsupported type + numberProperty: null + } + }; + + expect(true).to.be.true; + }); + }); + + describe('Customer reports', () => { + it('fixes issues/7813', () => { + interface TType { + prop: Record; + } + const update: UpdateData = {}; + const value: { [key: string]: { id: string } } = { + key: { id: '' } + }; + + update.prop = value; + + expect(true).to.be.true; + }); + + it('fixes node issues/1745#issuecomment-1289292949', () => { + interface TestType { + foo: { + [key: string]: { + bar: string; + }; + }; + } + // The intent of the function below is to test TypeScript compile and not execute. + async function _(docRef: DocumentReference): Promise { + const key = 'aKey'; + await updateDoc(docRef, { + [`foo.${key}.bar`]: 'test' + }); + } + + expect(true).to.be.true; + }); + + it('fixes node issues/1890', () => { + interface MyDoc { + nestedA: Record; + nestedB: Record; + } + + async function _( + db: Firestore, + docRef: DocumentReference + ): Promise { + const goodKey = 'nestedA.test'; + const badKey = 'nestedA.' + 'test'; + + await runTransaction(db, async t => { + t.update(docRef, { + [goodKey]: 3 + }); + }); + + await runTransaction(db, async t => { + t.update(docRef, { + [badKey]: 3 + }); + }); + } + + expect(true).to.be.true; + }); + }); }); describe('FirestoreTypeConverter', () => { From 1150700e901f40b7ae6f35f4dbb10d8a08acd0c4 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Mon, 18 Dec 2023 16:09:11 -0700 Subject: [PATCH 2/4] Create olive-geckos-argue.md --- .changeset/olive-geckos-argue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/olive-geckos-argue.md diff --git a/.changeset/olive-geckos-argue.md b/.changeset/olive-geckos-argue.md new file mode 100644 index 00000000000..b5a4990e20d --- /dev/null +++ b/.changeset/olive-geckos-argue.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +UpdateData allows indexed types or Record for T. From c57568db65d68a005d9c633e0064a87177bd923a Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Mon, 18 Dec 2023 16:50:05 -0700 Subject: [PATCH 3/4] Exports and doc-gen --- common/api-review/firestore.api.md | 9 +++++++-- packages/firestore/src/api.ts | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index ee4fcc842ff..7843afb82aa 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -78,6 +78,11 @@ export class Bytes { // @public export const CACHE_SIZE_UNLIMITED = -1; +// @public +export type ChildTypes = T extends Record ? { + [K in keyof T & string]: ChildTypes; +}[keyof T & string] | T : T; + // @public export type ChildUpdateFields = V extends Record ? AddPrefixToKeys> : never; @@ -413,7 +418,7 @@ export function namedQuery(firestore: Firestore, name: string): Promise> = UnionToIntersection<{ - [K in keyof T & string]: ChildUpdateFields; + [K in keyof T & string]: string extends K ? never : ChildUpdateFields; }[keyof T & string]>; // @public @@ -732,7 +737,7 @@ export interface Unsubscribe { // @public export type UpdateData = T extends Primitive ? T : T extends {} ? { - [K in keyof T]?: UpdateData | FieldValue; + [K in keyof T]?: string extends K ? PartialWithFieldValue> : UpdateData | FieldValue; } & NestedUpdateFields : Partial; // @public diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 510d95c8a89..52a6732310f 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -190,6 +190,7 @@ export { AbstractUserDataWriter } from './lite-api/user_data_writer'; export { AddPrefixToKeys, ChildUpdateFields, + ChildTypes, NestedUpdateFields, Primitive, UnionToIntersection From 8b5043f5e2e70514a35f87fc4aa375622fbded42 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 19 Dec 2023 17:32:56 -0700 Subject: [PATCH 4/4] lite api exports --- common/api-review/firestore-lite.api.md | 9 +++++++-- docs-devsite/firestore_.md | 17 +++++++++++++++-- docs-devsite/firestore_lite.md | 17 +++++++++++++++-- packages/firestore/lite/index.ts | 1 + 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 440fa488c1c..d884120efe6 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -75,6 +75,11 @@ export class Bytes { toUint8Array(): Uint8Array; } +// @public +export type ChildTypes = T extends Record ? { + [K in keyof T & string]: ChildTypes; +}[keyof T & string] | T : T; + // @public export type ChildUpdateFields = V extends Record ? AddPrefixToKeys> : never; @@ -259,7 +264,7 @@ export { LogLevel } // @public export type NestedUpdateFields> = UnionToIntersection<{ - [K in keyof T & string]: ChildUpdateFields; + [K in keyof T & string]: string extends K ? never : ChildUpdateFields; }[keyof T & string]>; // @public @@ -451,7 +456,7 @@ export type UnionToIntersection = (U extends unknown ? (k: U) => void : never // @public export type UpdateData = T extends Primitive ? T : T extends {} ? { - [K in keyof T]?: UpdateData | FieldValue; + [K in keyof T]?: string extends K ? PartialWithFieldValue> : UpdateData | FieldValue; } & NestedUpdateFields : Partial; // @public diff --git a/docs-devsite/firestore_.md b/docs-devsite/firestore_.md index ba7f6270621..ef3035d25d0 100644 --- a/docs-devsite/firestore_.md +++ b/docs-devsite/firestore_.md @@ -200,6 +200,7 @@ https://github.com/firebase/firebase-js-sdk | [AggregateFieldType](./firestore_.md#aggregatefieldtype) | The union of all AggregateField types that are supported by Firestore. | | [AggregateSpecData](./firestore_.md#aggregatespecdata) | A type whose keys are taken from an AggregateSpec, and whose values are the result of the aggregation performed by the corresponding AggregateField from the input AggregateSpec. | | [AggregateType](./firestore_.md#aggregatetype) | Union type representing the aggregate type to be performed. | +| [ChildTypes](./firestore_.md#childtypes) | For the given type, return a union type of T and the types of all child properties of T. | | [ChildUpdateFields](./firestore_.md#childupdatefields) | Helper for calculating the nested fields for a given type T1. This is needed to distribute union types such as undefined | {...} (happens for optional props) or {a: A} | {b: B}.In this use case, V is used to distribute the union types of T[K] on Record, since T[K] is evaluated as an expression and not distributed.See https://www.typescriptlang.org/docs/handbook/advanced-types.html\#distributive-conditional-types | | [DocumentChangeType](./firestore_.md#documentchangetype) | The type of a DocumentChange may be 'added', 'removed', or 'modified'. | | [FirestoreErrorCode](./firestore_.md#firestoreerrorcode) | The set of Firestore status codes. The codes are the same at the ones exposed by gRPC here: https://github.com/grpc/grpc/blob/master/doc/statuscodes.mdPossible values: - 'cancelled': The operation was cancelled (typically by the caller). - 'unknown': Unknown error or an error from a different error domain. - 'invalid-argument': Client specified an invalid argument. Note that this differs from 'failed-precondition'. 'invalid-argument' indicates arguments that are problematic regardless of the state of the system (e.g. an invalid field name). - 'deadline-exceeded': Deadline expired before operation could complete. For operations that change the state of the system, this error may be returned even if the operation has completed successfully. For example, a successful response from a server could have been delayed long enough for the deadline to expire. - 'not-found': Some requested document was not found. - 'already-exists': Some document that we attempted to create already exists. - 'permission-denied': The caller does not have permission to execute the specified operation. - 'resource-exhausted': Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system is out of space. - 'failed-precondition': Operation was rejected because the system is not in a state required for the operation's execution. - 'aborted': The operation was aborted, typically due to a concurrency issue like transaction aborts, etc. - 'out-of-range': Operation was attempted past the valid range. - 'unimplemented': Operation is not implemented or not supported/enabled. - 'internal': Internal errors. Means some invariants expected by underlying system has been broken. If you see one of these errors, something is very broken. - 'unavailable': The service is currently unavailable. This is most likely a transient condition and may be corrected by retrying with a backoff. - 'data-loss': Unrecoverable data loss or corruption. - 'unauthenticated': The request does not have valid authentication credentials for the operation. | @@ -2505,6 +2506,18 @@ Union type representing the aggregate type to be performed. export declare type AggregateType = 'count' | 'avg' | 'sum'; ``` +## ChildTypes + +For the given type, return a union type of T and the types of all child properties of T. + +Signature: + +```typescript +export declare type ChildTypes = T extends Record ? { + [K in keyof T & string]: ChildTypes; +}[keyof T & string] | T : T; +``` + ## ChildUpdateFields Helper for calculating the nested fields for a given type T1. This is needed to distribute union types such as `undefined | {...}` (happens for optional props) or `{a: A} | {b: B}`. @@ -2569,7 +2582,7 @@ For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, ```typescript export declare type NestedUpdateFields> = UnionToIntersection<{ - [K in keyof T & string]: ChildUpdateFields; + [K in keyof T & string]: string extends K ? never : ChildUpdateFields; }[keyof T & string]>; ``` @@ -2691,7 +2704,7 @@ Update data (for use with [updateDoc()](./firestore_.md#updatedoc_51a65e3)In this use case, V is used to distribute the union types of T[K] on Record, since T[K] is evaluated as an expression and not distributed.See https://www.typescriptlang.org/docs/handbook/advanced-types.html\#distributive-conditional-types | | [FirestoreErrorCode](./firestore_lite.md#firestoreerrorcode) | The set of Firestore status codes. The codes are the same at the ones exposed by gRPC here: https://github.com/grpc/grpc/blob/master/doc/statuscodes.mdPossible values: - 'cancelled': The operation was cancelled (typically by the caller). - 'unknown': Unknown error or an error from a different error domain. - 'invalid-argument': Client specified an invalid argument. Note that this differs from 'failed-precondition'. 'invalid-argument' indicates arguments that are problematic regardless of the state of the system (e.g. an invalid field name). - 'deadline-exceeded': Deadline expired before operation could complete. For operations that change the state of the system, this error may be returned even if the operation has completed successfully. For example, a successful response from a server could have been delayed long enough for the deadline to expire. - 'not-found': Some requested document was not found. - 'already-exists': Some document that we attempted to create already exists. - 'permission-denied': The caller does not have permission to execute the specified operation. - 'resource-exhausted': Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system is out of space. - 'failed-precondition': Operation was rejected because the system is not in a state required for the operation's execution. - 'aborted': The operation was aborted, typically due to a concurrency issue like transaction aborts, etc. - 'out-of-range': Operation was attempted past the valid range. - 'unimplemented': Operation is not implemented or not supported/enabled. - 'internal': Internal errors. Means some invariants expected by underlying system has been broken. If you see one of these errors, something is very broken. - 'unavailable': The service is currently unavailable. This is most likely a transient condition and may be corrected by retrying with a backoff. - 'data-loss': Unrecoverable data loss or corruption. - 'unauthenticated': The request does not have valid authentication credentials for the operation. | | [NestedUpdateFields](./firestore_lite.md#nestedupdatefields) | For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, 'bar.qux': T2}). Intersect them together to make a single map containing all possible keys that are all marked as optional | @@ -1612,6 +1613,18 @@ Union type representing the aggregate type to be performed. export declare type AggregateType = 'count' | 'avg' | 'sum'; ``` +## ChildTypes + +For the given type, return a union type of T and the types of all child properties of T. + +Signature: + +```typescript +export declare type ChildTypes = T extends Record ? { + [K in keyof T & string]: ChildTypes; +}[keyof T & string] | T : T; +``` + ## ChildUpdateFields Helper for calculating the nested fields for a given type T1. This is needed to distribute union types such as `undefined | {...}` (happens for optional props) or `{a: A} | {b: B}`. @@ -1646,7 +1659,7 @@ For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, ```typescript export declare type NestedUpdateFields> = UnionToIntersection<{ - [K in keyof T & string]: ChildUpdateFields; + [K in keyof T & string]: string extends K ? never : ChildUpdateFields; }[keyof T & string]>; ``` @@ -1746,7 +1759,7 @@ Update data (for use with [updateDoc()](./firestore_.md#updatedoc_51a65e3)