Allow type T in WithFieldValue<T> (#5675)

Author: Brian Chen

.changeset/short-mice-itch.md

@@ -0,0 +1,6 @@
+---
+'@firebase/firestore': minor
+'firebase': minor
+---
+
+Expanded `Firestore.WithFieldValue<T>` to include `T`. This allows developers to delegate `WithFieldValue<T>` inside wrappers of type `T` to avoid exposing Firebase types beyond Firebase-specific logic.

common/api-review/firestore-lite.api.md

@@ -204,9 +204,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir
 export type OrderByDirection = 'desc' | 'asc';
 
 // @public
-export type PartialWithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
+export type PartialWithFieldValue<T> = Partial<T> | (T extends Primitive ? T : T extends {} ? {
     [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue;
-} : Partial<T>;
+} : never);
 
 // @public
 export type Primitive = string | number | boolean | undefined | null;
@@ -352,9 +352,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value
 export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in';
 
 // @public
-export type WithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
+export type WithFieldValue<T> = T | (T extends Primitive ? T : T extends {} ? {
     [K in keyof T]: WithFieldValue<T[K]> | FieldValue;
-} : Partial<T>;
+} : never);
 
 // @public
 export class WriteBatch {

common/api-review/firestore.api.md

@@ -328,9 +328,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir
 export type OrderByDirection = 'desc' | 'asc';
 
 // @public
-export type PartialWithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
+export type PartialWithFieldValue<T> = Partial<T> | (T extends Primitive ? T : T extends {} ? {
     [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue;
-} : Partial<T>;
+} : never);
 
 // @public
 export interface PersistenceSettings {
@@ -504,9 +504,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value
 export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in';
 
 // @public
-export type WithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
+export type WithFieldValue<T> = T | (T extends Primitive ? T : T extends {} ? {
     [K in keyof T]: WithFieldValue<T[K]> | FieldValue;
-} : Partial<T>;
+} : never);
 
 // @public
 export class WriteBatch {

packages/firestore/src/lite-api/reference.ts

@@ -54,21 +54,25 @@ export interface DocumentData {
  * Similar to Typescript's `Partial<T>`, but allows nested fields to be
  * omitted and FieldValues to be passed in as property values.
  */
-export type PartialWithFieldValue<T> = T extends Primitive
-  ? T
-  : T extends {}
-  ? { [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue }
-  : Partial<T>;
+export type PartialWithFieldValue<T> =
+  | Partial<T>
+  | (T extends Primitive
+      ? T
+      : T extends {}
+      ? { [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue }
+      : never);
 
 /**
  * Allows FieldValues to be passed in as a property value while maintaining
  * type safety.
  */
-export type WithFieldValue<T> = T extends Primitive
-  ? T
-  : T extends {}
-  ? { [K in keyof T]: WithFieldValue<T[K]> | FieldValue }
-  : Partial<T>;
+export type WithFieldValue<T> =
+  | T
+  | (T extends Primitive
+      ? T
+      : T extends {}
+      ? { [K in keyof T]: WithFieldValue<T[K]> | FieldValue }
+      : never);
 
 /**
  * Update data (for use with {@link (updateDoc:1)}) that consists of field paths

packages/firestore/test/lite/integration.test.ts

@@ -1286,7 +1286,7 @@ describe('withConverter() support', () => {
       }
     };
 
-    describe('NestedPartial', () => {
+    describe('nested partial support', () => {
       const testConverterMerge = {
         toFirestore(
           testObj: PartialWithFieldValue<TestObject>,
@@ -1399,6 +1399,44 @@ describe('withConverter() support', () => {
           );
         });
       });
+
+      it('allows omitting fields', async () => {
+        return withTestDoc(async doc => {
+          const ref = doc.withConverter(testConverterMerge);
+
+          // Omit outer fields
+          await setDoc(
+            ref,
+            {
+              outerString: deleteField(),
+              nested: {
+                innerNested: {
+                  innerNestedNum: increment(1)
+                },
+                innerArr: arrayUnion(2),
+                timestamp: serverTimestamp()
+              }
+            },
+            { merge: true }
+          );
+
+          // Omit inner fields
+          await setDoc(
+            ref,
+            {
+              outerString: deleteField(),
+              outerArr: [],
+              nested: {
+                innerNested: {
+                  innerNestedNum: increment(1)
+                },
+                timestamp: serverTimestamp()
+              }
+            },
+            { merge: true }
+          );
+        });
+      });
     });
 
     describe('WithFieldValue', () => {
@@ -1421,7 +1459,7 @@ describe('withConverter() support', () => {
         });
       });
 
-      it('requires all fields to be present', async () => {
+      it('requires all outer fields to be present', async () => {
         return withTestDoc(async doc => {
           const ref = doc.withConverter(testConverter);
 
@@ -1440,6 +1478,24 @@ describe('withConverter() support', () => {
         });
       });
 
+      it('requires all nested fields to be present', async () => {
+        return withTestDoc(async doc => {
+          const ref = doc.withConverter(testConverter);
+
+          await setDoc(ref, {
+            outerString: 'foo',
+            outerArr: [],
+            // @ts-expect-error
+            nested: {
+              innerNested: {
+                innerNestedNum: increment(1)
+              },
+              timestamp: serverTimestamp()
+            }
+          });
+        });
+      });
+
       it('validates inner and outer fields', async () => {
         return withTestDoc(async doc => {
           const ref = doc.withConverter(testConverter);
@@ -1496,6 +1552,77 @@ describe('withConverter() support', () => {
           });
         });
       });
+
+      it('allows certain types but not others', () => {
+        const withTryCatch = async (fn: () => Promise<void>): Promise<void> => {
+          try {
+            await fn();
+          } catch {}
+        };
+
+        // These tests exist to establish which object types are allowed to be
+        // passed in by default when `T = DocumentData`. Some objects extend
+        // the Javascript `{}`, which is why they're allowed whereas others
+        // throw an error.
+        return withTestDoc(async doc => {
+          // @ts-expect-error
+          await withTryCatch(() => setDoc(doc, 1));
+          // @ts-expect-error
+          await withTryCatch(() => setDoc(doc, 'foo'));
+          // @ts-expect-error
+          await withTryCatch(() => setDoc(doc, false));
+          await withTryCatch(() => setDoc(doc, undefined));
+          await withTryCatch(() => setDoc(doc, null));
+          await withTryCatch(() => setDoc(doc, [0]));
+          await withTryCatch(() => setDoc(doc, new Set<string>()));
+          await withTryCatch(() => setDoc(doc, new Map<string, number>()));
+        });
+      });
+
+      describe('used as a type', () => {
+        class ObjectWrapper<T> {
+          withFieldValueT(value: WithFieldValue<T>): WithFieldValue<T> {
+            return value;
+          }
+
+          withPartialFieldValueT(
+            value: PartialWithFieldValue<T>
+          ): PartialWithFieldValue<T> {
+            return value;
+          }
+
+          // Wrapper to avoid having Firebase types in non-Firebase code.
+          withT(value: T): void {
+            this.withFieldValueT(value);
+          }
+
+          // Wrapper to avoid having Firebase types in non-Firebase code.
+          withPartialT(value: Partial<T>): void {
+            this.withPartialFieldValueT(value);
+          }
+        }
+
+        it('supports passing in the object as `T`', () => {
+          interface Foo {
+            id: string;
+            foo: number;
+          }
+          const foo = new ObjectWrapper<Foo>();
+          foo.withFieldValueT({ id: '', foo: increment(1) });
+          foo.withPartialFieldValueT({ foo: increment(1) });
+          foo.withT({ id: '', foo: 1 });
+          foo.withPartialT({ foo: 1 });
+        });
+
+        it('does not allow primitive types to use FieldValue', () => {
+          type Bar = number;
+          const bar = new ObjectWrapper<Bar>();
+          // @ts-expect-error
+          bar.withFieldValueT(increment(1));
+          // @ts-expect-error
+          bar.withPartialFieldValueT(increment(1));
+        });
+      });
     });
 
     describe('UpdateData', () => {