feat: use update_transform when possible

dev/src/document.ts

@@ -452,16 +452,6 @@ export class DocumentSnapshot<T = DocumentData> {
     return (fields as ApiMapValue)[components[0]];
   }
 
-  /**
-   * Checks whether this DocumentSnapshot contains any fields.
-   *
-   * @private
-   * @return {boolean}
-   */
-  get isEmpty(): boolean {
-    return this._fieldsProto === undefined || isEmpty(this._fieldsProto);
-  }
-
   /**
    * Convert a document snapshot to the Firestore 'Document' Protobuf.
    *
@@ -971,29 +961,19 @@ export class DocumentTransform<T = DocumentData> {
   }
 
   /**
-   * Converts a document transform to the Firestore 'DocumentTransform' Proto.
+   * Converts a document transform to the Firestore 'FieldTransform' Proto.
    *
    * @private
    * @param serializer The Firestore serializer
-   * @returns A Firestore 'DocumentTransform' Proto or 'null' if this transform
-   * is empty.
+   * @returns A list of Firestore 'FieldTransform' Protos
    */
-  toProto(serializer: Serializer): api.IWrite | null {
-    if (this.isEmpty) {
-      return null;
-    }
-
-    const fieldTransforms: api.DocumentTransform.IFieldTransform[] = [];
+  toProto(serializer: Serializer): api.DocumentTransform.IFieldTransform[] {
+    const fieldTransforms = [];
     for (const [path, transform] of this.transforms) {
       fieldTransforms.push(transform.toProto(serializer, path));
     }
 
-    return {
-      transform: {
-        document: this.ref.formattedName,
-        fieldTransforms,
-      },
-    };
+    return fieldTransforms;
   }
 }
 

dev/src/write-batch.ts

@@ -107,7 +107,6 @@ export class WriteResult {
 // TODO(mrschmidt): Replace with api.IWrite
 interface WriteOp {
   write?: api.IWrite | null;
-  transform?: api.IWrite | null;
   precondition?: api.IPrecondition | null;
 }
 
@@ -194,12 +193,13 @@ export class WriteBatch {
 
     const op = () => {
       const document = DocumentSnapshot.fromObject(documentRef, firestoreData);
-      const write =
-        !document.isEmpty || transform.isEmpty ? document.toProto() : null;
+      const write = document.toProto();
+      if (!transform.isEmpty) {
+        write.updateTransforms = transform.toProto(this._serializer);
+      }
 
       return {
         write,
-        transform: transform.toProto(this._serializer),
         precondition: precondition.toProto(),
       };
     };
@@ -324,20 +324,18 @@ export class WriteBatch {
         documentMask = DocumentMask.fromObject(firestoreData);
       }
 
-      const hasDocumentData = !document.isEmpty || !documentMask!.isEmpty;
-
-      let write;
+      const write = document.toProto();
+      if (!transform.isEmpty) {
+        write.updateTransforms = transform.toProto(this._serializer);
+      }
 
-      if (!mergePaths && !mergeLeaves) {
-        write = document.toProto();
-      } else if (hasDocumentData || transform.isEmpty) {
-        write = document.toProto()!;
+      const hasMerge = mergePaths || mergeLeaves;
+      if (hasMerge) {
         write.updateMask = documentMask!.toProto();
       }
 
       return {
         write,
-        transform: transform.toProto(this._serializer),
       };
     };
 
@@ -474,16 +472,13 @@ export class WriteBatch {
 
     const op = () => {
       const document = DocumentSnapshot.fromUpdateMap(documentRef, updateMap);
-      let write: api.IWrite | null = null;
-
-      if (!document.isEmpty || !documentMask.isEmpty) {
-        write = document.toProto();
-        write!.updateMask = documentMask.toProto();
+      const write = document.toProto();
+      write!.updateMask = documentMask.toProto();
+      if (!transform.isEmpty) {
+        write!.updateTransforms = transform.toProto(this._serializer);
       }
-
       return {
         write,
-        transform: transform.toProto(this._serializer),
         precondition: precondition.toProto(),
       };
     };
@@ -557,22 +552,13 @@ export class WriteBatch {
     request.writes = [];
 
     for (const req of writes) {
-      assert(
-        req.write || req.transform,
-        'Either a write or transform must be set'
-      );
+      assert(req.write, 'A write must be set');
 
       if (req.precondition) {
-        (req.write || req.transform)!.currentDocument = req.precondition;
-      }
-
-      if (req.write) {
-        request.writes.push(req.write);
+        req.write!.currentDocument = req.precondition;
       }
 
-      if (req.transform) {
-        request.writes.push(req.transform);
-      }
+      request.writes.push(req.write!);
     }
 
     logger(
@@ -602,23 +588,7 @@ export class WriteBatch {
 
           const commitTime = Timestamp.fromProto(resp.commitTime!);
 
-          let offset = 0;
-
-          for (let i = 0; i < writes.length; ++i) {
-            const writeRequest = writes[i];
-
-            // Don't return two write results for a write that contains a
-            // transform, as the fact that we have to split one write
-            // operation into two distinct write requests is an implementation
-            // detail.
-            if (writeRequest.write && writeRequest.transform) {
-              // The document transform is always sent last and produces the
-              // latest update time.
-              ++offset;
-            }
-
-            const writeResult = resp.writeResults[i + offset];
-
+          for (const writeResult of resp.writeResults) {
             writeResults.push(
               new WriteResult(
                 writeResult.updateTime

dev/system-test/firestore.ts

@@ -1975,19 +1975,28 @@ describe('Transaction class', () => {
   it('has update() method', () => {
     const ref = randomCol.doc('doc');
     return ref
-      .set({foo: 'bar'})
+      .set({
+        boo: ['ghost', 'sebastian'],
+        moo: 'chicken',
+      })
       .then(() => {
         return firestore.runTransaction(updateFunction => {
           return updateFunction.get(ref).then(() => {
-            updateFunction.update(ref, {foo: 'foobar'});
+            updateFunction.update(ref, {
+              boo: FieldValue.arrayRemove('sebastian'),
+              moo: 'cow',
+            });
           });
         });
       })
       .then(() => {
         return ref.get();
       })
       .then(doc => {
-        expect(doc.get('foo')).to.equal('foobar');
+        expect(doc.data()).to.deep.equal({
+          boo: ['ghost'],
+          moo: 'cow',
+        });
       });
   });
 

dev/test/document.ts

@@ -913,6 +913,8 @@ describe('set document', () => {
         requestEquals(
           request,
           set({
+            document: document('documentId'),
+            mask: updateMask(),
             transforms: [serverTimestamp('a'), serverTimestamp('b.c')],
           })
         );
@@ -941,7 +943,7 @@ describe('set document', () => {
             transforms: [serverTimestamp('a'), serverTimestamp('b.c')],
           })
         );
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -1121,7 +1123,7 @@ describe('set document', () => {
             ],
           })
         );
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -1354,6 +1356,7 @@ describe('create document', () => {
         requestEquals(
           request,
           create({
+            document: document('documentId'),
             transforms: [
               serverTimestamp('field'),
               serverTimestamp('map.field'),
@@ -1458,7 +1461,7 @@ describe('update document', () => {
             mask: updateMask('a', 'foo'),
           })
         );
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -1474,7 +1477,13 @@ describe('update document', () => {
   it('skips write for single field transform', () => {
     const overrides: ApiOverride = {
       commit: request => {
-        requestEquals(request, update({transforms: [serverTimestamp('a')]}));
+        requestEquals(
+          request,
+          update({
+            document: document('documentId'),
+            transforms: [serverTimestamp('a')],
+          })
+        );
         return response(writeResult(1));
       },
     };

dev/test/field-value.ts

@@ -108,7 +108,7 @@ describe('FieldValue.arrayUnion()', () => {
 
         requestEquals(request, expectedRequest);
 
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -162,7 +162,7 @@ describe('FieldValue.increment()', () => {
           ],
         });
         requestEquals(request, expectedRequest);
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -205,7 +205,7 @@ describe('FieldValue.arrayRemove()', () => {
         });
         requestEquals(request, expectedRequest);
 
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 
@@ -240,7 +240,7 @@ describe('FieldValue.serverTimestamp()', () => {
         });
         requestEquals(request, expectedRequest);
 
-        return response(writeResult(2));
+        return response(writeResult(1));
       },
     };
 

dev/test/util/helpers.ts

@@ -89,27 +89,23 @@ export function verifyInstance(firestore: Firestore): Promise<void> {
 }
 
 function write(
-  document: api.IDocument | null,
+  document: api.IDocument,
   mask: api.IDocumentMask | null,
   transforms: api.DocumentTransform.IFieldTransform[] | null,
   precondition: api.IPrecondition | null
 ): api.ICommitRequest {
   const writes: api.IWrite[] = [];
+  const update = Object.assign({}, document);
+  delete update.updateTime;
+  delete update.createTime;
+  writes.push({update});
 
-  if (document) {
-    const update = Object.assign({}, document);
-    delete update.updateTime;
-    delete update.createTime;
-    writes.push({update});
-    if (mask) {
-      writes[0].updateMask = mask;
-    }
+  if (mask) {
+    writes[0].updateMask = mask;
   }
 
   if (transforms) {
-    writes.push({
-      transform: {document: DOCUMENT_NAME, fieldTransforms: transforms},
-    });
+    writes[0].updateTransforms = transforms;
   }
 
   if (precondition) {
@@ -124,47 +120,37 @@ export function updateMask(...fieldPaths: string[]): api.IDocumentMask {
 }
 
 export function set(opts: {
-  document?: api.IDocument;
+  document: api.IDocument;
   transforms?: api.DocumentTransform.IFieldTransform[];
   mask?: api.IDocumentMask;
 }): api.ICommitRequest {
   return write(
-    opts.document || null,
+    opts.document,
     opts.mask || null,
     opts.transforms || null,
-    null
+    /** precondition= */ null
   );
 }
 
 export function update(opts: {
-  document?: api.IDocument;
+  document: api.IDocument;
   transforms?: api.DocumentTransform.IFieldTransform[];
   mask?: api.IDocumentMask;
   precondition?: api.IPrecondition;
 }): api.ICommitRequest {
   const precondition = opts.precondition || {exists: true};
   const mask = opts.mask || updateMask();
-  return write(
-    opts.document || null,
-    mask,
-    opts.transforms || null,
-    precondition
-  );
+  return write(opts.document, mask, opts.transforms || null, precondition);
 }
 
 export function create(opts: {
-  document?: api.IDocument;
+  document: api.IDocument;
   transforms?: api.DocumentTransform.IFieldTransform[];
   mask?: api.IDocumentMask;
 }): api.ICommitRequest {
-  return write(
-    opts.document || null,
-    /* updateMask */ null,
-    opts.transforms || null,
-    {
-      exists: false,
-    }
-  );
+  return write(opts.document, /* updateMask */ null, opts.transforms || null, {
+    exists: false,
+  });
 }
 
 function value(value: string | api.IValue): api.IValue {