Replace usage of transform with updateTransforms

.changeset/rich-birds-wink.md

@@ -0,0 +1,5 @@
+---
+'@firebase/firestore': minor
+---
+
+Field tranforms performed within a write no longer count as additional operations. However, a field transform on its own still counts as an operation.

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

@@ -314,8 +314,8 @@ export function setDoc<T>(
     options
   );
 
-  const mutations = parsed.toMutations(reference._key, Precondition.none());
-  return executeWrite(firestore, mutations);
+  const mutation = parsed.toMutation(reference._key, Precondition.none());
+  return executeWrite(firestore, [mutation]);
 }
 
 /**
@@ -394,11 +394,8 @@ export function updateDoc(
     );
   }
 
-  const mutations = parsed.toMutations(
-    reference._key,
-    Precondition.exists(true)
-  );
-  return executeWrite(firestore, mutations);
+  const mutation = parsed.toMutation(reference._key, Precondition.exists(true));
+  return executeWrite(firestore, [mutation]);
 }
 
 /**
@@ -448,8 +445,8 @@ export function addDoc<T>(
     {}
   );
 
-  const mutations = parsed.toMutations(docRef._key, Precondition.exists(false));
-  return executeWrite(firestore, mutations).then(() => docRef);
+  const mutation = parsed.toMutation(docRef._key, Precondition.exists(false));
+  return executeWrite(firestore, [mutation]).then(() => docRef);
 }
 
 // TODO(firestorexp): Make sure these overloads are tested via the Firestore

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

@@ -176,9 +176,9 @@ export interface DocumentChange<T = DocumentData> {
  * access will return 'undefined'. You can use the `exists()` method to
  * explicitly verify a document's existence.
  */
-export class DocumentSnapshot<
-  T = DocumentData
-> extends LiteDocumentSnapshot<T> {
+export class DocumentSnapshot<T = DocumentData> extends LiteDocumentSnapshot<
+  T
+> {
   private readonly _firestoreImpl: FirebaseFirestore;
 
   /**
@@ -291,9 +291,9 @@ export class DocumentSnapshot<
  * `exists` property will always be true and `data()` will never return
  * 'undefined'.
  */
-export class QueryDocumentSnapshot<
-  T = DocumentData
-> extends DocumentSnapshot<T> {
+export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<
+  T
+> {
   /**
    * Retrieves all fields in the document as an `Object`.
    *

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

@@ -1478,10 +1478,9 @@ export function setDoc<T>(
   );
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(reference._key, Precondition.none())
-  );
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(reference._key, Precondition.none())
+  ]);
 }
 
 /**
@@ -1569,10 +1568,9 @@ export function updateDoc(
   }
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(reference._key, Precondition.exists(true))
-  );
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(reference._key, Precondition.exists(true))
+  ]);
 }
 
 /**
@@ -1634,10 +1632,9 @@ export function addDoc<T>(
   );
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(docRef._key, Precondition.exists(false))
-  ).then(() => docRef);
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(docRef._key, Precondition.exists(false))
+  ]).then(() => docRef);
 }
 
 /**

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

@@ -211,9 +211,9 @@ export class DocumentSnapshot<T = DocumentData> {
  * `exists` property will always be true and `data()` will never return
  * 'undefined'.
  */
-export class QueryDocumentSnapshot<
-  T = DocumentData
-> extends DocumentSnapshot<T> {
+export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<
+  T
+> {
   /**
    * Retrieves all fields in the document as an `Object`.
    *

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

@@ -111,9 +111,7 @@ export class WriteBatch {
       ref._converter !== null,
       options
     );
-    this._mutations = this._mutations.concat(
-      parsed.toMutations(ref._key, Precondition.none())
-    );
+    this._mutations.push(parsed.toMutation(ref._key, Precondition.none()));
     return this;
   }
 
@@ -186,8 +184,8 @@ export class WriteBatch {
       );
     }
 
-    this._mutations = this._mutations.concat(
-      parsed.toMutations(ref._key, Precondition.exists(true))
+    this._mutations.push(
+      parsed.toMutation(ref._key, Precondition.exists(true))
     );
     return this;
   }

packages/firestore/src/api/user_data_reader.ts

@@ -17,13 +17,13 @@
 
 import {
   DocumentData,
-  SetOptions,
-  FieldPath as PublicFieldPath
+  FieldPath as PublicFieldPath,
+  SetOptions
 } from '@firebase/firestore-types';
 
 import {
-  Value as ProtoValue,
-  MapValue as ProtoMapValue
+  MapValue as ProtoMapValue,
+  Value as ProtoValue
 } from '../protos/firestore_proto_api';
 import { Timestamp } from './timestamp';
 import { DatabaseId } from '../core/database_info';
@@ -34,8 +34,7 @@ import {
   Mutation,
   PatchMutation,
   Precondition,
-  SetMutation,
-  TransformMutation
+  SetMutation
 } from '../model/mutation';
 import { FieldPath as InternalFieldPath } from '../model/path';
 import { debugAssert, fail } from '../util/assert';
@@ -79,19 +78,23 @@ export class ParsedSetData {
     readonly fieldTransforms: FieldTransform[]
   ) {}
 
-  toMutations(key: DocumentKey, precondition: Precondition): Mutation[] {
-    const mutations = [] as Mutation[];
+  toMutation(key: DocumentKey, precondition: Precondition): Mutation {
     if (this.fieldMask !== null) {
-      mutations.push(
-        new PatchMutation(key, this.data, this.fieldMask, precondition)
+      return new PatchMutation(
+        key,
+        this.data,
+        this.fieldMask,
+        precondition,
+        this.fieldTransforms
       );
     } else {
-      mutations.push(new SetMutation(key, this.data, precondition));
-    }
-    if (this.fieldTransforms.length > 0) {
-      mutations.push(new TransformMutation(key, this.fieldTransforms));
+      return new SetMutation(
+        key,
+        this.data,
+        precondition,
+        this.fieldTransforms
+      );
     }
-    return mutations;
   }
 }
 
@@ -103,14 +106,14 @@ export class ParsedUpdateData {
     readonly fieldTransforms: FieldTransform[]
   ) {}
 
-  toMutations(key: DocumentKey, precondition: Precondition): Mutation[] {
-    const mutations = [
-      new PatchMutation(key, this.data, this.fieldMask, precondition)
-    ] as Mutation[];
-    if (this.fieldTransforms.length > 0) {
-      mutations.push(new TransformMutation(key, this.fieldTransforms));
-    }
-    return mutations;
+  toMutation(key: DocumentKey, precondition: Precondition): Mutation {
+    return new PatchMutation(
+      key,
+      this.data,
+      this.fieldMask,
+      precondition,
+      this.fieldTransforms
+    );
   }
 }
 

packages/firestore/src/core/transaction.ts

@@ -81,21 +81,21 @@ export class Transaction {
   }
 
   set(key: DocumentKey, data: ParsedSetData): void {
-    this.write(data.toMutations(key, this.precondition(key)));
+    this.write(data.toMutation(key, this.precondition(key)));
     this.writtenDocs.add(key.toString());
   }
 
   update(key: DocumentKey, data: ParsedUpdateData): void {
     try {
-      this.write(data.toMutations(key, this.preconditionForUpdate(key)));
+      this.write(data.toMutation(key, this.preconditionForUpdate(key)));
     } catch (e) {
       this.lastWriteError = e;
     }
     this.writtenDocs.add(key.toString());
   }
 
   delete(key: DocumentKey): void {
-    this.write([new DeleteMutation(key, this.precondition(key))]);
+    this.write(new DeleteMutation(key, this.precondition(key)));
     this.writtenDocs.add(key.toString());
   }
 
@@ -193,9 +193,9 @@ export class Transaction {
     }
   }
 
-  private write(mutations: Mutation[]): void {
+  private write(mutation: Mutation): void {
     this.ensureCommitNotCalled();
-    this.mutations = this.mutations.concat(mutations);
+    this.mutations.push(mutation);
   }
 
   private ensureCommitNotCalled(): void {

packages/firestore/src/local/local_serializer.ts

@@ -194,6 +194,30 @@ export function fromDbMutationBatch(
   const baseMutations = (dbBatch.baseMutations || []).map(m =>
     fromMutation(localSerializer.remoteSerializer, m)
   );
+
+  // Squash old transform mutations into existing patch or set mutations.
+  // The replacement of representing `transforms` with `update_transforms`
+  // on the SDK means that old `transform` mutations stored in IndexedDB need
+  // to be updated to `update_transforms`.
+  // TODO(b/174608374): Remove this code once we perform a schema migration.
+  let i = dbBatch.mutations.length - 1;
+  while (i >= 0) {
+    const mutationProto = dbBatch.mutations[i];
+    if (mutationProto?.transform !== undefined) {
+      debugAssert(
+        i >= 1 &&
+          dbBatch.mutations[i - 1].transform === undefined &&
+          dbBatch.mutations[i - 1].update !== undefined,
+        'TransformMutation should be preceded by a patch or set mutation'
+      );
+      const mutationToJoin = dbBatch.mutations[i - 1];
+      mutationToJoin.updateTransforms = mutationProto.transform.fieldTransforms;
+      dbBatch.mutations.splice(i, 1);
+      --i;
+    }
+    --i;
+  }
+
   const mutations = dbBatch.mutations.map(m =>
     fromMutation(localSerializer.remoteSerializer, m)
   );