Making .dotChanges a method

dev/src/reference.js

@@ -729,22 +729,6 @@ export class QuerySnapshot {
     return this._materializedDocs;
   }
 
-  /**
-   * An array of all changes in this QuerySnapshot.
-   *
-   * @type {Array.<DocumentChange>}
-   * @name QuerySnapshot#docChanges
-   * @readonly
-   */
-  get docChanges() {
-    if (this._materializedChanges) {
-      return this._materializedChanges;
-    }
-    this._materializedChanges = this._changes();
-    this._changes = null;
-    return this._materializedChanges;
-  }
-
   /**
    * True if there are no documents in the QuerySnapshot.
    *
@@ -801,6 +785,32 @@ export class QuerySnapshot {
     return this._readTime;
   }
 
+  /**
+   * Returns an array of the documents changes since the last snapshot. If
+   * this is the first snapshot, all documents will be in the list as added
+   * changes.
+   *
+   * @return {Array.<DocumentChange>}
+   *
+   * @example
+   * let query = firestore.collection('col').where('foo', '==', 'bar');
+   *
+   * query.onSnapshot(querySnapshot => {
+   *   let changes = querySnapshot.docChanges();
+   *   for (let change of changes) {
+   *     console.log(`A document was ${change.type}.`);
+   *   }
+   * });
+   */
+  docChanges() {
+    if (this._materializedChanges) {
+      return this._materializedChanges;
+    }
+    this._materializedChanges = this._changes();
+    this._changes = null;
+    return this._materializedChanges;
+  }
+
   /**
    * Enumerates all of the documents in the QuerySnapshot.
    *
@@ -858,16 +868,38 @@ export class QuerySnapshot {
       // If we have only materialized the documents, we compare them first.
       return (
           isArrayEqual(this.docs, other.docs) &&
-          isArrayEqual(this.docChanges, other.docChanges));
+          isArrayEqual(this.docChanges(), other.docChanges()));
     }
 
     // Otherwise, we compare the changes first as we expect there to be fewer.
     return (
-        isArrayEqual(this.docChanges, other.docChanges) &&
+        isArrayEqual(this.docChanges(), other.docChanges()) &&
         isArrayEqual(this.docs, other.docs));
   }
 }
 
+// TODO: As of v0.17.0, we're changing docChanges from an array into a method.
+// Because this is a runtime breaking change and somewhat subtle (both Array and
+// Function have a .length, etc.), we'll replace commonly-used properties
+// (including Symbol.iterator) to throw a custom error message. By our v1.0
+// release, we should remove this code.
+function throwDocChangesMethodError() {
+  throw new Error(
+      'QuerySnapshot.docChanges has been changed from a property into a ' +
+      'method, so usages like "querySnapshot.docChanges" should become ' +
+      '"querySnapshot.docChanges()"');
+}
+
+const docChangesPropertiesToOverride = [
+  'length', 'forEach', 'map',
+  ...(typeof Symbol !== 'undefined' ? [Symbol.iterator] : [])
+];
+docChangesPropertiesToOverride.forEach(property => {
+  Object.defineProperty(
+      QuerySnapshot.prototype.docChanges, property,
+      {get: () => throwDocChangesMethodError()});
+});
+
 /**
  * A Query refers to a query which you can read or stream from. You can also
  * construct refined Query objects by adding filters and ordering.

dev/system-test/firestore.js

@@ -1120,16 +1120,17 @@ describe('Query class', function() {
         assert.equal(actual.docs[i].ref.id, expected.docs[i].ref.id);
         assert.deepStrictEqual(actual.docs[i].data(), expected.docs[i].data());
       }
-      assert.equal(actual.docChanges.length, expected.docChanges.length);
+      const actualDocChanges = actual.docChanges();
+      assert.equal(actualDocChanges.length, expected.docChanges.length);
       for (i = 0; i < expected.docChanges.length; i++) {
-        assert.equal(actual.docChanges[i].type, expected.docChanges[i].type);
+        assert.equal(actualDocChanges.type, expected.docChanges[i].type);
         assert.equal(
-            actual.docChanges[i].doc.ref.id, expected.docChanges[i].doc.ref.id);
+            actualDocChanges[i].doc.ref.id, expected.docChanges[i].doc.ref.id);
         assert.deepStrictEqual(
-            actual.docChanges[i].doc.data(), expected.docChanges[i].doc.data());
-        assert.ok(is.defined(actual.docChanges[i].doc.readTime));
-        assert.ok(is.defined(actual.docChanges[i].doc.createTime));
-        assert.ok(is.defined(actual.docChanges[i].doc.updateTime));
+            actualDocChanges[i].doc.data(), expected.docChanges[i].doc.data());
+        assert.ok(is.defined(actualDocChanges[i].doc.readTime));
+        assert.ok(is.defined(actualDocChanges[i].doc.createTime));
+        assert.ok(is.defined(actualDocChanges[i].doc.updateTime));
       }
       assert.ok(is.defined(actual.readTime));
     };

dev/test/query.js

@@ -495,7 +495,7 @@ describe('query interface', function() {
         assert.ok(results.readTime.isEqual(new Firestore.Timestamp(5, 6)));
         assert.equal('first', results.docs[0].get('first'));
         assert.equal('second', results.docs[1].get('second'));
-        assert.equal(2, results.docChanges.length);
+        assert.equal(2, results.docChanges().length);
 
         let count = 0;
 
@@ -590,6 +590,29 @@ describe('query interface', function() {
           });
     });
   });
+
+  it('throws if QuerySnapshot.docChanges() is used as a property', function() {
+    const overrides = {
+      runQuery: (request) => {
+        requestEquals(request);
+        return stream(document('first'), document('second'));
+      }
+    };
+
+    return createInstance(overrides).then(firestore => {
+      let query = firestore.collection('collectionId');
+      return query.get().then(snapshot => {
+        assert.throws(() => {
+          snapshot.docChanges.forEach(() => {});
+        }, /QuerySnapshot.docChanges has been changed from a property into a method/);
+
+        assert.throws(() => {
+          for (const doc of snapshot.docChanges) {
+          }
+        }, /QuerySnapshot.docChanges has been changed from a property into a method/);
+      });
+    });
+  });
 });
 
 describe('where() interface', function() {

dev/test/typescript.ts

@@ -268,7 +268,7 @@ xdescribe('firestore.d.ts', function() {
   it('has typings for QuerySnapshot', () => {
     collRef.get().then((snapshot: QuerySnapshot) => {
       const query: Query = snapshot.query;
-      const docChanges: DocumentChange[] = snapshot.docChanges;
+      const docChanges: DocumentChange[] = snapshot.docChanges();
       const docs: QueryDocumentSnapshot[] = snapshot.docs;
       const size: number = snapshot.size;
       const empty: boolean = snapshot.empty;
@@ -283,7 +283,7 @@ xdescribe('firestore.d.ts', function() {
 
   it('has typings for DocumentChange', () => {
     collRef.get().then((snapshot: QuerySnapshot) => {
-      const docChange: DocumentChange = snapshot.docChanges[0];
+      const docChange: DocumentChange = snapshot.docChanges()[0];
       const doc: QueryDocumentSnapshot = docChange.doc;
       const oldIndex: number = docChange.oldIndex;
       const newIndex: number = docChange.newIndex;

dev/test/watch.js

@@ -57,25 +57,27 @@ const docsEqual = function(actual, expected) {
 const snapshotsEqual = function(lastSnapshot, version, actual, expected) {
   let localDocs = [].concat(lastSnapshot.docs);
 
-  assert.equal(actual.docChanges.length, expected.docChanges.length);
+  const actualDocChanges = actual.docChanges();
+
+  assert.equal(actualDocChanges.length, expected.docChanges.length);
   for (let i = 0; i < expected.docChanges.length; i++) {
-    assert.equal(actual.docChanges[i].type, expected.docChanges[i].type);
+    assert.equal(actualDocChanges[i].type, expected.docChanges[i].type);
     assert.equal(
-        actual.docChanges[i].doc.ref.id, expected.docChanges[i].doc.ref.id);
+        actualDocChanges[i].doc.ref.id, expected.docChanges[i].doc.ref.id);
     assert.deepStrictEqual(
-        actual.docChanges[i].doc.data(), expected.docChanges[i].doc.data());
+        actualDocChanges[i].doc.data(), expected.docChanges[i].doc.data());
     let readVersion =
-        actual.docChanges[i].type === 'removed' ? version - 1 : version;
-    assert.ok(actual.docChanges[i].doc.readTime.isEqual(
+        actualDocChanges[i].type === 'removed' ? version - 1 : version;
+    assert.ok(actualDocChanges[i].doc.readTime.isEqual(
         new Firestore.Timestamp(0, readVersion)));
 
-    if (actual.docChanges[i].oldIndex !== -1) {
-      localDocs.splice(actual.docChanges[i].oldIndex, 1);
+    if (actualDocChanges[i].oldIndex !== -1) {
+      localDocs.splice(actualDocChanges[i].oldIndex, 1);
     }
 
-    if (actual.docChanges[i].newIndex !== -1) {
+    if (actualDocChanges[i].newIndex !== -1) {
       localDocs.splice(
-          actual.docChanges[i].newIndex, 0, actual.docChanges[i].doc);
+          actualDocChanges[i].newIndex, 0, actualDocChanges[i].doc);
     }
   }
 
@@ -84,7 +86,7 @@ const snapshotsEqual = function(lastSnapshot, version, actual, expected) {
   assert.ok(actual.readTime.isEqual(new Firestore.Timestamp(0, version)));
   assert.equal(actual.size, expected.docs.length);
 
-  return {docs: actual.docs, docChanges: actual.docChanges};
+  return {docs: actual.docs, docChanges: actualDocChanges};
 };
 
 /*
@@ -1891,17 +1893,17 @@ describe('Query watch', function() {
                         watchHelper.sendDoc(doc1, {foo: 'a'});
                       }).then(snapshot => {
                  firstSnapshot = snapshot;
-                 assert.ok(snapshot.docChanges[0].isEqual(
-                     firstSnapshot.docChanges[0]));
+                 assert.ok(snapshot.docChanges()[0].isEqual(
+                     firstSnapshot.docChanges()[0]));
                });
              })
           .then(() => initialSnapshot(snapshot => {
                   return nextSnapshot(snapshot, () => {
                            watchHelper.sendDoc(doc1, {foo: 'b'});
                          }).then(snapshot => {
                     assert.ok(!snapshot.isEqual(firstSnapshot));
-                    assert.ok(!snapshot.docChanges[0].isEqual(
-                        firstSnapshot.docChanges[0]));
+                    assert.ok(!snapshot.docChanges()[0].isEqual(
+                        firstSnapshot.docChanges()[0]));
                   });
                 }));
     });

types/firestore.d.ts

@@ -931,13 +931,6 @@ declare namespace FirebaseFirestore {
      */
     readonly query: Query;
 
-    /**
-     * An array of the documents that changed since the last snapshot. If this
-     * is the first snapshot, all documents will be in the list as added
-     * changes.
-     */
-    readonly docChanges: DocumentChange[];
-
     /** An array of all the documents in the QuerySnapshot. */
     readonly docs: QueryDocumentSnapshot[];
 
@@ -950,6 +943,13 @@ declare namespace FirebaseFirestore {
     /** The time this query snapshot was obtained. */
     readonly readTime: Timestamp;
 
+    /**
+     * Returns an array of the documents changes since the last snapshot. If
+     * this is the first snapshot, all documents will be in the list as added
+     * changes.
+     */
+    docChanges(): DocumentChange[];
+
     /**
      * Enumerates all of the documents in the QuerySnapshot.
      *