Don't drop empty objects during merge (#1202)

Author: schmidt-sebastian

packages/firestore/CHANGELOG.md

@@ -4,6 +4,8 @@
 - [changed] Changed `get()` to only make 1 attempt to reach the backend before
   returning cached data, potentially reducing delays while offline. Previously
   it would make 2 attempts, to work around a backend bug.
+- [fixed] Fixed an issue that caused us to drop empty objects from calls to
+  `set(..., { merge: true })`.
 
 # 0.7.3
 - [changed] Changed the internal handling for locally updated documents that

packages/firestore/src/api/user_data_converter.ts

@@ -539,15 +539,25 @@ export class UserDataConverter {
 
   private parseObject(obj: Dict<AnyJs>, context: ParseContext): FieldValue {
     let result = new SortedMap<string, FieldValue>(primitiveComparator);
-    objUtils.forEach(obj, (key: string, val: AnyJs) => {
-      const parsedValue = this.parseData(
-        val,
-        context.childContextForField(key)
-      );
-      if (parsedValue != null) {
-        result = result.insert(key, parsedValue);
+
+    if (objUtils.isEmpty(obj)) {
+      // If we encounter an empty object, we explicitly add it to the update
+      // mask to ensure that the server creates a map entry.
+      if (context.path && context.path.length > 0) {
+        context.fieldMask.push(context.path);
       }
-    });
+    } else {
+      objUtils.forEach(obj, (key: string, val: AnyJs) => {
+        const parsedValue = this.parseData(
+          val,
+          context.childContextForField(key)
+        );
+        if (parsedValue != null) {
+          result = result.insert(key, parsedValue);
+        }
+      });
+    }
+
     return new ObjectValue(result);
   }
 

packages/firestore/src/model/mutation.ts

@@ -449,11 +449,13 @@ export class PatchMutation extends Mutation {
 
   private patchObject(data: ObjectValue): ObjectValue {
     for (const fieldPath of this.fieldMask.fields) {
-      const newValue = this.data.field(fieldPath);
-      if (newValue !== undefined) {
-        data = data.set(fieldPath, newValue);
-      } else {
-        data = data.delete(fieldPath);
+      if (!fieldPath.isEmpty()) {
+        const newValue = this.data.field(fieldPath);
+        if (newValue !== undefined) {
+          data = data.set(fieldPath, newValue);
+        } else {
+          data = data.delete(fieldPath);
+        }
       }
     }
     return data;

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

@@ -188,6 +188,34 @@ apiDescribe('Database', persistence => {
     });
   });
 
+  it('can merge empty object', async () => {
+    await withTestDoc(persistence, async doc => {
+      const accumulator = new EventsAccumulator<firestore.DocumentSnapshot>();
+      const unsubscribe = doc.onSnapshot(accumulator.storeEvent);
+      await accumulator
+        .awaitEvent()
+        .then(() => doc.set({}))
+        .then(() => accumulator.awaitEvent())
+        .then(docSnapshot => expect(docSnapshot.data()).to.be.deep.equal({}))
+        .then(() => doc.set({ a: {} }, { mergeFields: ['a'] }))
+        .then(() => accumulator.awaitEvent())
+        .then(docSnapshot =>
+          expect(docSnapshot.data()).to.be.deep.equal({ a: {} })
+        )
+        .then(() => doc.set({ b: {} }, { merge: true }))
+        .then(() => accumulator.awaitEvent())
+        .then(docSnapshot =>
+          expect(docSnapshot.data()).to.be.deep.equal({ a: {}, b: {} })
+        )
+        .then(() => doc.get({ source: 'server' }))
+        .then(docSnapshot => {
+          expect(docSnapshot.data()).to.be.deep.equal({ a: {}, b: {} });
+        });
+
+      unsubscribe();
+    });
+  });
+
   it('can delete field using merge', () => {
     return withTestDoc(persistence, doc => {
       const initialData = {