Comment fixes

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_schema.ts

@@ -447,8 +447,8 @@ export class DbRemoteDocument {
   constructor(
     /**
      * Set to an instance of DbUnknownDocument if the data for a document is
-     * not known, but yet we no the document exist (e.g. it had a successful
-     * update applied to it)
+     * not known, but it is known that a document exist at the specified version
+     * (e.g. it had a successful update applied to it)
      */
     public unknownDocument: DbUnknownDocument | null | undefined,
     /**

packages/firestore/src/local/local_serializer.ts

@@ -71,23 +71,23 @@ export class LocalSerializer {
     if (maybeDoc instanceof Document) {
       const doc = this.remoteSerializer.toDocument(maybeDoc);
       const hasCommittedMutations = maybeDoc.hasCommittedMutations;
-      return new DbRemoteDocument(null, null, doc, hasCommittedMutations);
+      return new DbRemoteDocument(/* unknownDocument= */null, /* noDocument= */null, doc, hasCommittedMutations);
     } else if (maybeDoc instanceof NoDocument) {
       const path = maybeDoc.key.path.toArray();
       const readTime = this.toDbTimestamp(maybeDoc.version);
       return new DbRemoteDocument(
-        null,
+        /* unknownDocument= */null,
         new DbNoDocument(path, readTime),
-        null,
+          /* document= */null,
         /* hasCommittedMutations= */ false
       );
     } else if (maybeDoc instanceof UnknownDocument) {
       const path = maybeDoc.key.path.toArray();
       const readTime = this.toDbTimestamp(maybeDoc.version);
       return new DbRemoteDocument(
         new DbUnknownDocument(path, readTime),
-        null,
-        null,
+          /* noDocument= */null,
+          /* document= */null,
         /* hasCommittedMutations= */ false
       );
     } else {

packages/firestore/src/model/document.ts

@@ -38,6 +38,10 @@ export abstract class MaybeDocument {
     return DocumentKey.comparator(d1.key, d2.key);
   }
 
+  /**
+   * Whether this document had a local mutation applied that has not yet been
+   * acknowledged by Watch.
+   */
   abstract hasPendingWrites(): boolean;
 
   abstract isEqual(other: MaybeDocument | null | undefined): boolean;
@@ -137,8 +141,8 @@ export class NoDocument extends MaybeDocument {
 }
 
 /**
- * A class representing a document whose data is unknown (e.g. a document that
- * was updated without a known base version).
+ * A class representing an existing document whose data is unknown (e.g. a
+ * document that was updated without a known base document).
  */
 export class UnknownDocument extends MaybeDocument {
   constructor(key: DocumentKey, version: SnapshotVersion) {

packages/firestore/src/model/mutation.ts

@@ -376,20 +376,18 @@ export class PatchMutation extends Mutation {
       'Transform results received by PatchMutation.'
     );
 
-    const version = mutationResult.version;
-
     // TODO(mcg): Relax enforcement of this precondition
     //
     // We shouldn't actually enforce the precondition since it already passed on
     // the backend, but we may not have a local version of the document to
     // patch, so we use the precondition to prevent incorrectly putting a
     // partial document into our cache.
     if (!this.precondition.isValidFor(maybeDoc)) {
-      return new UnknownDocument(this.key, version);
+      return new UnknownDocument(this.key, mutationResult.version);
     }
 
     const newData = this.patchDocument(maybeDoc);
-    return new Document(this.key, version, newData, {
+    return new Document(this.key, mutationResult.version, newData, {
       hasCommittedMutations: true
     });
   }

packages/firestore/test/unit/specs/listen_spec.test.ts

@@ -82,7 +82,7 @@ describeSpec('Listens:', [], () => {
 
   specTest("Doesn't include unknown documents in cached result", [], () => {
     const query1 = Query.atPath(path('collection'));
-    const docA = doc(
+    const existingDoc = doc(
       'collection/exists',
       0,
       { key: 'a' },
@@ -93,7 +93,7 @@ describeSpec('Listens:', [], () => {
       .userPatches('collection/unknown', { key: 'b' })
       .userListens(query1)
       .expectEvents(query1, {
-        added: [docA],
+        added: [existingDoc],
         fromCache: true,
         hasPendingWrites: true
       });
@@ -378,13 +378,12 @@ describeSpec('Listens:', [], () => {
 
     return (
       spec()
-        // Disable GC so the cache persists across listens.
-        .withGCEnabled(false)
         .userListens(query1)
         .watchAcksFull(query1, 1000, docAv1)
         .expectEvents(query1, { added: [docAv1] })
         .userDeletes('collection/a')
         .expectEvents(query1, { removed: [docAv1] })
+        // We have a unacknowledged delete and ignore the Watch update.
         .watchSends({ affects: [query1] }, docAv2)
         .watchSnapshots(2000)
         // The write stream acks our delete at version 4000.