Almost address feedback

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_persistence.ts

@@ -1225,10 +1225,7 @@ export class IndexedDbLruDelegate implements ReferenceDelegate, LruDelegate {
               // Our size accounting requires us to read all documents before
               // removing them.
               return changeBuffer.getEntry(txn, docKey).next(() => {
-                changeBuffer.removeEntry(
-                  docKey,
-                  SnapshotVersion.forDeletedDoc()
-                );
+                changeBuffer.removeEntry(docKey);
                 return documentTargetStore(txn).delete(sentinelKey(docKey));
               });
             }

packages/firestore/src/local/indexeddb_remote_document_cache.ts

@@ -110,7 +110,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
   }
 
   /**
-   * Updates the current size.
+   * Updates the current cache size.
    *
    * Callers to `addEntry()` and `removeEntry()` *must* call this afterwards to update the
    * cache's metadata.

packages/firestore/src/local/memory_persistence.ts

@@ -277,7 +277,7 @@ export class MemoryEagerDelegate implements ReferenceDelegate {
       (key: DocumentKey) => {
         return this.isReferenced(txn, key).next(isReferenced => {
           if (!isReferenced) {
-            changeBuffer.removeEntry(key, SnapshotVersion.forDeletedDoc());
+            changeBuffer.removeEntry(key);
           }
         });
       }
@@ -417,7 +417,7 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate {
       return this.isPinned(txn, key, upperBound).next(isPinned => {
         if (!isPinned) {
           count++;
-          changeBuffer.removeEntry(key, SnapshotVersion.forDeletedDoc());
+          changeBuffer.removeEntry(key);
         }
       });
     });

packages/firestore/src/local/remote_document_change_buffer.ts

@@ -80,7 +80,7 @@ export abstract class RemoteDocumentChangeBuffer {
   protected get readTime(): SnapshotVersion {
     assert(
       this._readTime !== undefined,
-      'Read time is not set. Did you call addEntry/removeEntry?'
+      'Read time is not set. All removeEntry() calls must include a readTime if `trackRemovals` is used.'
     );
     return this._readTime!;
   }
@@ -103,9 +103,11 @@ export abstract class RemoteDocumentChangeBuffer {
    * You can only remove documents that have already been retrieved via
    * `getEntry()/getEntries()` (enforced via IndexedDbs `apply()`).
    */
-  removeEntry(key: DocumentKey, readTime: SnapshotVersion): void {
+  removeEntry(key: DocumentKey, readTime?: SnapshotVersion): void {
     this.assertNotApplied();
-    this.readTime = readTime;
+    if (readTime) {
+      this.readTime = readTime;
+    }
     this.changes.set(key, null);
   }
 

packages/firestore/test/unit/local/remote_document_change_buffer.test.ts

@@ -73,7 +73,7 @@ describe('RemoteDocumentChangeBuffer', () => {
     const newADoc = doc('coll/a', 43, { new: 'data' });
     buffer.addEntry(newADoc, newADoc.version);
     expect(await buffer.getEntry(key('coll/a'))).to.not.be.null;
-    buffer.removeEntry(newADoc.key, newADoc.version);
+    buffer.removeEntry(newADoc.key);
     expect(await buffer.getEntry(key('coll/a'))).to.be.null;
   });
 
@@ -94,7 +94,7 @@ describe('RemoteDocumentChangeBuffer', () => {
 
   it('can apply changes with removal', async () => {
     expectEqual(await buffer.getEntry(key('coll/a')), INITIAL_DOC);
-    buffer.removeEntry(INITIAL_DOC.key, INITIAL_DOC.version);
+    buffer.removeEntry(INITIAL_DOC.key);
     // Reading directly against the cache should still yield the old result.
     expectEqual(await cache.getEntry(key('coll/a')), INITIAL_DOC);
 

packages/firestore/test/unit/local/test_remote_document_change_buffer.ts

@@ -35,8 +35,8 @@ export class TestRemoteDocumentChangeBuffer {
     this.buffer.addEntry(maybeDocument, readTime);
   }
 
-  removeEntry(key: DocumentKey, readTime: SnapshotVersion): void {
-    this.buffer.removeEntry(key, readTime);
+  removeEntry(key: DocumentKey): void {
+    this.buffer.removeEntry(key);
   }
 
   getEntry(documentKey: DocumentKey): Promise<MaybeDocument | null> {