Call toFirestore() once in add()

Author: Brian Chen

packages/firestore/exp/src/api/transaction.ts

@@ -29,7 +29,8 @@ import { Transaction as InternalTransaction } from '../../../src/core/transactio
 import { validateReference } from '../../../lite/src/api/write_batch';
 import { getDatastore } from '../../../lite/src/api/components';
 
-export class Transaction extends LiteTransaction
+export class Transaction
+  extends LiteTransaction
   implements firestore.Transaction {
   // This class implements the same logic as the Transaction API in the Lite SDK
   // but is subclassed in order to return its own DocumentSnapshot types.

packages/firestore/lite/src/api/field_value.ts

@@ -30,7 +30,8 @@ import { ParseContext } from '../../../src/api/user_data_reader';
 import { FieldTransform } from '../../../src/model/mutation';
 
 /** The public FieldValue class of the lite API. */
-export abstract class FieldValue extends SerializableFieldValue
+export abstract class FieldValue
+  extends SerializableFieldValue
   implements firestore.FieldValue {}
 
 /**

packages/firestore/src/api/database.ts

@@ -2379,8 +2379,24 @@ export class CollectionReference<T = DocumentData>
       ? this._converter.toFirestore(value)
       : value;
     validateArgType('CollectionReference.add', 'object', 1, convertedValue);
+
+    // Call set() with the converted value directly to avoid calling toFirestore() a second time.
     const docRef = this.doc();
-    return docRef.set(value).then(() => docRef);
+    const defaultConverter: FirestoreDataConverter<DocumentData> = {
+      toFirestore(modelObject: DocumentData): DocumentData {
+        return modelObject;
+      },
+      fromFirestore(
+        snapshot: QueryDocumentSnapshot,
+        options: SnapshotOptions
+      ): DocumentData {
+        return snapshot.data(options)!;
+      }
+    };
+    return docRef
+      .withConverter(defaultConverter)
+      .set(convertedValue)
+      .then(() => docRef);
   }
 
   withConverter<U>(

packages/firestore/test/integration/api/validation.test.ts

@@ -589,6 +589,18 @@ apiDescribe('Validation:', (persistence: boolean) => {
         });
     });
 
+    validationIt(
+      persistence,
+      'validates CollectionReference.add with correct function name',
+      db => {
+        const coll = db.collection('foo');
+
+        expect(() => coll.add(new TestClass('foo'))).to.throw(
+          'Function CollectionReference.add() called with invalid data.'
+        );
+      }
+    );
+
     validationIt(persistence, 'must not contain undefined.', db => {
       // Note: This test uses the default setting for `ignoreUndefinedProperties`.
       return expectWriteToFail(