Remove Symbol.iterator (#1330)

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_schema.ts

@@ -164,7 +164,9 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
               );
               const batch = this.serializer.fromDbMutationBatch(dbBatch);
 
-              return removeMutationBatch(txn, queue.userId, batch).next();
+              return removeMutationBatch(txn, queue.userId, batch).next(
+                () => {}
+              );
             });
           });
       });

packages/firestore/src/local/memory_persistence.ts

@@ -247,8 +247,9 @@ export class MemoryEagerDelegate implements ReferenceDelegate {
           // Since this is the eager delegate and memory persistence,
           // we don't care about the size of documents. We don't track
           // the size of the cache for eager GC.
-          return cache.removeEntry(txn, key).next();
+          return cache.removeEntry(txn, key).next(() => {});
         }
+        return PersistencePromise.resolve();
       });
     });
   }
@@ -326,7 +327,7 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate {
   ): PersistencePromise<void> {
     return PersistencePromise.forEach(
       this.orphanedSequenceNumbers,
-      ({ key, value: sequenceNumber }) => {
+      (key, sequenceNumber) => {
         // Pass in the exact sequence number as the upper bound so we know it won't be pinned by
         // being too recent.
         return this.isPinned(txn, key, sequenceNumber).next(isPinned => {

packages/firestore/src/local/memory_remote_document_cache.ts

@@ -136,7 +136,7 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
     transaction: PersistenceTransaction,
     f: (key: DocumentKey) => PersistencePromise<void>
   ): PersistencePromise<void> {
-    return PersistencePromise.forEach(this.docs, entry => f(entry.key));
+    return PersistencePromise.forEach(this.docs, key => f(key));
   }
 
   getNewDocumentChanges(

packages/firestore/src/local/persistence_promise.ts

@@ -170,58 +170,33 @@ export class PersistencePromise<T> {
 
   static waitFor(
     // tslint:disable-next-line:no-any Accept all Promise types in waitFor().
-    all: Iterable<PersistencePromise<any>>
+    all: { forEach: (cb: ((el: PersistencePromise<any>) => void)) => void }
   ): PersistencePromise<void> {
-    const it = all[Symbol.iterator]();
     return new PersistencePromise<void>((resolve, reject) => {
       let expectedCount = 0;
       let resolvedCount = 0;
+      let done = false;
 
-      let result = it.next();
-      while (!result.done) {
+      all.forEach(element => {
         ++expectedCount;
-        result.value.next(
+        element.next(
           () => {
             ++resolvedCount;
-            if (result.done && resolvedCount === expectedCount) {
+            if (done && resolvedCount === expectedCount) {
               resolve();
             }
           },
           err => reject(err)
         );
-        result = it.next();
-      }
+      });
 
+      done = true;
       if (resolvedCount === expectedCount) {
         resolve();
       }
     });
   }
 
-  static map<R>(all: Iterable<PersistencePromise<R>>): PersistencePromise<R[]> {
-    const results: R[] = [];
-    const promises: Array<PersistencePromise<void>> = [];
-
-    const it = all[Symbol.iterator]();
-    let result = it.next();
-    let count = 0;
-    while (!result.done) {
-      const value = result.value;
-      const index = count;
-
-      promises.push(
-        value.next(val => {
-          results[index] = val;
-        })
-      );
-
-      result = it.next();
-      ++count;
-    }
-
-    return PersistencePromise.waitFor(promises).next(() => results);
-  }
-
   /**
    * Given an array of predicate functions that asynchronously evaluate to a
    * boolean, implements a short-circuiting `or` between the results. Predicates
@@ -246,22 +221,19 @@ export class PersistencePromise<T> {
 
   /**
    * Given an iterable, call the given function on each element in the
-   * collection and wait for all of the resulting concurrent
-   * PersistencePromises to resolve.
+   * collection and wait for all of the resulting concurrent PersistencePromises
+   * to resolve.
    */
-  static forEach<K>(
-    collection: Iterable<K>,
-    f: (item: K) => PersistencePromise<void>
+  static forEach<R, S>(
+    collection: { forEach: ((cb: ((r: R, s?: S) => void)) => void) },
+    f:
+      | ((r: R, s: S) => PersistencePromise<void>)
+      | ((r: R) => PersistencePromise<void>)
   ): PersistencePromise<void> {
-    const it = collection[Symbol.iterator]();
-
     const promises: Array<PersistencePromise<void>> = [];
-    let result = it.next();
-    while (!result.done) {
-      const value = result.value;
-      promises.push(f(value));
-      result = it.next();
-    }
+    collection.forEach((r, s) => {
+      promises.push(f.call(this, r, s));
+    });
     return this.waitFor(promises);
   }
 }

packages/firestore/src/util/obj_map.ts

@@ -107,12 +107,4 @@ export class ObjectMap<KeyType extends Equatable<KeyType>, ValueType> {
   isEmpty(): boolean {
     return objUtil.isEmpty(this.inner);
   }
-
-  [Symbol.iterator](): Iterator<{ key: KeyType; value: ValueType }> {
-    // We don't care about the keys, all of the actual keys are in the
-    // arrays that are the values of the inner object.
-    const entries: Array<{ key: KeyType; value: ValueType }> = [];
-    this.forEach((key, value) => entries.push({ key, value }));
-    return entries[Symbol.iterator]();
-  }
 }

packages/firestore/src/util/sorted_map.ts

@@ -168,21 +168,6 @@ export class SortedMap<K, V> {
   getReverseIteratorFrom(key: K): SortedMapIterator<K, V> {
     return new SortedMapIterator<K, V>(this.root, key, this.comparator, true);
   }
-
-  [Symbol.iterator](): Iterator<Entry<K, V>> {
-    const it = this.getIterator();
-    return {
-      next(): IteratorResult<Entry<K, V>> {
-        if (it.hasNext()) {
-          return { done: false, value: it.getNext() };
-        } else {
-          // The TypeScript typings don't allow `undefined` for Iterator<T>,
-          // so we return an empty object instead.
-          return { done: true, value: {} as Entry<K, V> };
-        }
-      }
-    };
-  }
 } // end SortedMap
 
 // An iterator over an LLRBNode.

packages/firestore/src/util/sorted_set.ts

@@ -154,21 +154,6 @@ export class SortedSet<T> {
     return 'SortedSet(' + result.toString() + ')';
   }
 
-  [Symbol.iterator](): Iterator<T> {
-    const it = this.data.getIterator();
-    return {
-      next(): IteratorResult<T> {
-        if (it.hasNext()) {
-          return { done: false, value: it.getNext().key };
-        } else {
-          // The TypeScript typings don't allow `undefined` for Iterator<T>,
-          // so we return an empty object instead.
-          return { done: true, value: {} as T };
-        }
-      }
-    };
-  }
-
   private copy(data: SortedMap<T, boolean>): SortedSet<T> {
     const result = new SortedSet(this.comparator);
     result.data = data;

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

@@ -16,7 +16,6 @@
 
 import { expect } from 'chai';
 import { PersistencePromise } from '../../../src/local/persistence_promise';
-import { Deferred } from '../../../src/util/promise';
 
 import * as chai from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
@@ -236,40 +235,10 @@ describe('PersistencePromise', () => {
       if (success) {
         return PersistencePromise.resolve();
       } else {
-        return PersistencePromise.reject(new Error('rejected'));
+        return PersistencePromise.reject<void>(new Error('rejected'));
       }
     }).toPromise();
 
     return expect(p).to.be.eventually.rejectedWith('rejected');
   });
-
-  it('maintains order for map()', async () => {
-    const deferred = new Deferred<void>();
-
-    const pending = new PersistencePromise<string>(resolve => {
-      return deferred.promise.then(() => resolve('first'));
-    });
-    const resolved = PersistencePromise.resolve('second');
-
-    const p = PersistencePromise.map([pending, resolved]).next(results => {
-      expect(results).to.deep.eq(['first', 'second']);
-      return PersistencePromise.resolve();
-    });
-
-    setImmediate(() => {
-      deferred.resolve();
-    });
-
-    await p.toPromise();
-  });
-
-  it('propagates error for map()', () => {
-    const resolved = PersistencePromise.resolve('resolved');
-    const rejected: PersistencePromise<string> = PersistencePromise.reject(
-      new Error('rejected')
-    );
-
-    const p = PersistencePromise.map([resolved, rejected]).toPromise();
-    return expect(p).to.be.eventually.rejectedWith('rejected');
-  });
 });

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

@@ -105,7 +105,7 @@ describe('IndexedDbRemoteDocumentCache', () => {
   ): PersistencePromise<void> {
     const changeBuffer = cache.newChangeBuffer();
     return PersistencePromise.forEach(docs, doc =>
-      changeBuffer.getEntry(txn, doc.key).next()
+      changeBuffer.getEntry(txn, doc.key).next(() => {})
     ).next(() => {
       for (const doc of docs) {
         changeBuffer.addEntry(doc);

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

@@ -51,7 +51,7 @@ export abstract class TestRemoteDocumentCache {
       txn => {
         const changeBuffer = this.cache.newChangeBuffer();
         return PersistencePromise.forEach(maybeDocuments, maybeDocument =>
-          changeBuffer.getEntry(txn, maybeDocument.key).next()
+          changeBuffer.getEntry(txn, maybeDocument.key).next(() => {})
         ).next(() => {
           for (const maybeDocument of maybeDocuments) {
             changeBuffer.addEntry(maybeDocument);

packages/firestore/test/unit/util/obj_map.test.ts

@@ -110,39 +110,4 @@ describe('ObjectMap', () => {
     expect(map.get(k2)).to.equal(undefined);
     expect(map.get(k3)).to.equal(undefined);
   });
-
-  it('can iterate', () => {
-    const map = new ObjectMap<TestKey, string>(o => o.mapKey);
-    const k1 = new TestKey(4, 4);
-    const k2 = new TestKey(5, 5);
-    // Test a collision
-    const k3 = new TestKey(5, 6);
-    // Test an overwrite
-    const k4 = new TestKey(-12354, -12354);
-    const k5 = new TestKey(-12354, -12354);
-    map.set(k1, 'foo');
-    map.set(k2, 'bar');
-    map.set(k3, 'blah');
-    map.set(k4, 'baz');
-    map.set(k5, 'boo');
-
-    const expectedKeys: TestKey[] = [];
-    map.forEach(key => {
-      expectedKeys.push(key);
-    });
-
-    const iterated: Array<{ key: TestKey; value: string }> = [];
-    const it = map[Symbol.iterator]();
-    let result = it.next();
-    while (!result.done) {
-      iterated.push(result.value);
-      result = it.next();
-    }
-
-    for (const entry of iterated) {
-      expect(expectedKeys.indexOf(entry.key)).to.not.equal(-1);
-      expect(map.get(entry.key)).to.equal(entry.value);
-    }
-    expect(iterated.length).to.equal(expectedKeys.length);
-  });
 });