rxfire: Support collection groups & change detection fixess (#2223)

* rxfire was comparing documents based off ids, this was fine before collection group queries. Here I've switched to `ref.isEqual` like I have done in AngularFire.
* When a document is modified, we were only replacing the single doc in the array; this would not always trip change detection in frameworks as `===` would still equal true on the top level. Splicing it creates a new array, which will fail `===` and trip change detection.
* Further, this was an issue with the other operations as we were using splice. I'm now slicing to clone the array before modifying.
* NIT: Visual Studio Code's settings, which are committed, were not setup to track the workspace's version of Typescript. This tripped me up a little at first.

Author: jamesdaniels

.vscode/settings.json

@@ -8,5 +8,6 @@
 
     // Exclude installed dependencies from searches too
     "**/node_modules": true
-  }
+  },
+  "typescript.tsdk": "node_modules/typescript/lib"
 }

packages/rxfire/firestore/collection/index.ts

@@ -18,7 +18,7 @@
 import { firestore } from 'firebase/app';
 import { fromCollectionRef } from '../fromRef';
 import { Observable, MonoTypeOperatorFunction } from 'rxjs';
-import { map, filter, scan } from 'rxjs/operators';
+import { map, filter, scan, distinctUntilChanged } from 'rxjs/operators';
 import { snapToData } from '../document';
 
 const ALL_EVENTS: firestore.DocumentChangeType[] = [
@@ -56,6 +56,16 @@ const filterEmpty = filter(
   (changes: firestore.DocumentChange[]) => changes.length > 0
 );
 
+/**
+ * Splice arguments on top of a sliced array, to break top-level ===
+ * this is useful for change-detection
+ */
+function sliceAndSplice<T>(original: T[], start: number, deleteCount: number, ...args: T[]): T[] {
+  const returnArray = original.slice();
+  returnArray.splice(start, deleteCount, ...args);
+  return returnArray;
+}
+
 /**
  * Creates a new sorted array from a new change.
  * @param combined
@@ -69,35 +79,37 @@ function processIndividualChange(
     case 'added':
       if (
         combined[change.newIndex] &&
-        combined[change.newIndex].doc.id === change.doc.id
+        combined[change.newIndex].doc.ref.isEqual(change.doc.ref)
       ) {
         // Skip duplicate emissions. This is rare.
         // TODO: Investigate possible bug in SDK.
       } else {
-        combined.splice(change.newIndex, 0, change);
+        return sliceAndSplice(combined, change.newIndex, 0, change);
       }
       break;
     case 'modified':
       if (
         combined[change.oldIndex] == null ||
-        combined[change.oldIndex].doc.id === change.doc.id
+        combined[change.oldIndex].doc.ref.isEqual(change.doc.ref)
       ) {
         // When an item changes position we first remove it
         // and then add it's new position
         if (change.oldIndex !== change.newIndex) {
-          combined.splice(change.oldIndex, 1);
-          combined.splice(change.newIndex, 0, change);
+          const copiedArray = combined.slice();
+          copiedArray.splice(change.oldIndex, 1);
+          copiedArray.splice(change.newIndex, 0, change);
+          return copiedArray;
         } else {
-          combined[change.newIndex] = change;
+          return sliceAndSplice(combined, change.newIndex, 1, change);
         }
       }
       break;
     case 'removed':
       if (
         combined[change.oldIndex] &&
-        combined[change.oldIndex].doc.id === change.doc.id
+        combined[change.oldIndex].doc.ref.isEqual(change.doc.ref)
       ) {
-        combined.splice(change.oldIndex, 1);
+        return sliceAndSplice(combined, change.oldIndex, 1);
       }
       break;
     default: // ignore
@@ -167,7 +179,8 @@ export function sortedChanges(
         changes: firestore.DocumentChange[]
       ) => processDocumentChanges(current, changes, events),
       []
-    )
+    ),
+    distinctUntilChanged()
   );
 }
 

packages/rxfire/test/firestore.test.ts

@@ -165,12 +165,15 @@ describe('RxFire Firestore', () => {
         take(1)
       );
 
+      let previousData: Array<{}>;
+
       addedChanges.subscribe(data => {
         const expectedNames = [
           { name: 'David', type: 'added' },
           { name: 'Shannon', type: 'added' }
         ];
         expect(data).to.eql(expectedNames);
+        previousData = data;
         davidDoc.update({ name: 'David!' });
       });
 
@@ -180,6 +183,7 @@ describe('RxFire Firestore', () => {
           { name: 'Shannon', type: 'added' }
         ];
         expect(data).to.eql(expectedNames);
+        expect(data === previousData).to.eql(false);
         done();
       });
     });