Merge branch 'master' into ddb-chrome-version-warnings

Author: DellaBitta

.changeset/itchy-chicken-cry.md

@@ -0,0 +1,2 @@
+---
+---

.changeset/long-rats-walk.md

@@ -0,0 +1,2 @@
+---
+---

.changeset/yellow-houses-happen.md

@@ -0,0 +1,6 @@
+---
+'@firebase/firestore': patch
+'firebase': patch
+---
+
+Update the `isEqual` function for arrayUnion, arrayRemove and increment.

packages/firestore/src/lite-api/user_data_reader.ts

@@ -20,7 +20,7 @@ import {
   FieldPath as PublicFieldPath,
   SetOptions
 } from '@firebase/firestore-types';
-import { Compat, getModularInstance } from '@firebase/util';
+import { Compat, deepEqual, getModularInstance } from '@firebase/util';
 
 import { ParseContext } from '../api/parse_context';
 import { DatabaseId } from '../core/database_info';
@@ -525,13 +525,15 @@ export class ArrayUnionFieldValueImpl extends FieldValue {
   }
 
   isEqual(other: FieldValue): boolean {
-    // TODO(mrschmidt): Implement isEquals
-    return this === other;
+    return (
+      other instanceof ArrayUnionFieldValueImpl &&
+      deepEqual(this._elements, other._elements)
+    );
   }
 }
 
 export class ArrayRemoveFieldValueImpl extends FieldValue {
-  constructor(methodName: string, readonly _elements: unknown[]) {
+  constructor(methodName: string, private readonly _elements: unknown[]) {
     super(methodName);
   }
 
@@ -549,8 +551,10 @@ export class ArrayRemoveFieldValueImpl extends FieldValue {
   }
 
   isEqual(other: FieldValue): boolean {
-    // TODO(mrschmidt): Implement isEquals
-    return this === other;
+    return (
+      other instanceof ArrayRemoveFieldValueImpl &&
+      deepEqual(this._elements, other._elements)
+    );
   }
 }
 
@@ -568,8 +572,10 @@ export class NumericIncrementFieldValueImpl extends FieldValue {
   }
 
   isEqual(other: FieldValue): boolean {
-    // TODO(mrschmidt): Implement isEquals
-    return this === other;
+    return (
+      other instanceof NumericIncrementFieldValueImpl &&
+      this._operand === other._operand
+    );
   }
 }
 

packages/firestore/test/integration/api/aggregation.test.ts

@@ -132,14 +132,16 @@ apiDescribe('Count queries', persistence => {
   // and will, therefore, never fail in this situation.
   // eslint-disable-next-line no-restricted-properties
   (USE_EMULATOR ? it.skip : it)(
-    'getCountFromServer error message is good if missing index',
+    'getCountFromServer error message contains console link if missing index',
     () => {
       return withEmptyTestCollection(persistence, async coll => {
         const query_ = query(
           coll,
           where('key1', '==', 42),
           where('key2', '<', 42)
         );
+        // TODO(b/316359394) Remove the special logic for non-default databases
+        // once cl/582465034 is rolled out to production.
         if (coll.firestore._databaseId.isDefaultDatabase) {
           await expect(
             getCountFromServer(query_)
@@ -342,14 +344,16 @@ apiDescribe('Aggregation queries', persistence => {
   // and will, therefore, never fail in this situation.
   // eslint-disable-next-line no-restricted-properties
   (USE_EMULATOR ? it.skip : it)(
-    'getAggregateFromServer error message is good if missing index',
+    'getAggregateFromServer error message contains console link good if missing index',
     () => {
       return withEmptyTestCollection(persistence, async coll => {
         const query_ = query(
           coll,
           where('key1', '==', 42),
           where('key2', '<', 42)
         );
+        // TODO(b/316359394) Remove the special logic for non-default databases
+        // once cl/582465034 is rolled out to production.
         if (coll.firestore._databaseId.isDefaultDatabase) {
           await expect(
             getAggregateFromServer(query_, {

packages/firestore/test/lite/integration.test.ts

@@ -883,6 +883,16 @@ describe('FieldValue', () => {
     expect(deleteField().isEqual(deleteField())).to.be.true;
     expect(serverTimestamp().isEqual(serverTimestamp())).to.be.true;
     expect(deleteField().isEqual(serverTimestamp())).to.be.false;
+    expect(arrayUnion().isEqual(arrayUnion())).to.be.true;
+    expect(arrayUnion('a').isEqual(arrayUnion('a'))).to.be.true;
+    expect(arrayUnion('a').isEqual(arrayUnion('b'))).to.be.false;
+    expect(arrayUnion('a', 'b').isEqual(arrayUnion('b', 'a'))).to.be.false;
+    expect(arrayRemove().isEqual(arrayRemove())).to.be.true;
+    expect(arrayRemove('a').isEqual(arrayRemove('a'))).to.be.true;
+    expect(arrayRemove('a').isEqual(arrayRemove('b'))).to.be.false;
+    expect(arrayRemove('a', 'b').isEqual(arrayRemove('b', 'a'))).to.be.false;
+    expect(increment(1).isEqual(increment(1))).to.be.true;
+    expect(increment(1).isEqual(increment(2))).to.be.false;
   });
 
   it('support instanceof checks', () => {
@@ -2396,14 +2406,16 @@ describe('Count queries', () => {
   // and will, therefore, never fail in this situation.
   // eslint-disable-next-line no-restricted-properties
   (USE_EMULATOR ? it.skip : it)(
-    'getCount error message is good if missing index',
+    'getCount error message contains console link if missing index',
     () => {
       return withTestCollection(async coll => {
         const query_ = query(
           coll,
           where('key1', '==', 42),
           where('key2', '<', 42)
         );
+        // TODO(b/316359394) Remove the special logic for non-default databases
+        // once cl/582465034 is rolled out to production.
         if (coll.firestore._databaseId.isDefaultDatabase) {
           await expect(getCount(query_)).to.be.eventually.rejectedWith(
             /index.*https:\/\/console\.firebase\.google\.com/
@@ -2703,14 +2715,16 @@ describe('Aggregate queries', () => {
   // and will, therefore, never fail in this situation.
   // eslint-disable-next-line no-restricted-properties
   (USE_EMULATOR ? it.skip : it)(
-    'getAggregate error message is good if missing index',
+    'getAggregate error message contains console link if missing index',
     () => {
       return withTestCollection(async coll => {
         const query_ = query(
           coll,
           where('key1', '==', 42),
           where('key2', '<', 42)
         );
+        // TODO(b/316359394) Remove the special logic for non-default databases
+        // once cl/582465034 is rolled out to production.
         if (coll.firestore._databaseId.isDefaultDatabase) {
           await expect(
             getAggregate(query_, {