Empty objects clear updated fields using set with merge

[mchyb]

Environment

• Operating System version: any
• Browser version: Chrome 70.0.3538.77 (64-bit)
• Firebase SDK version: 5.5.7
• Firebase Product: firestore

Steps to reproduce:

1. Create a document in any collection with a field of object type:

{
  "field": {
    "nested": "value"
  }
}

2. Update the field with empty object using set with merge flag:

ref.set({ field: {} }, { merge: true });

Expected result of operation:

{
  "field": {
    "nested": "value",
  }
}

Actual result:

{
  "field": {}
}

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[mikelehen]

[UPDATE: This reply isn't very useful because I accidentally read the expected / actual behavior in the repro backwards. Leaving for historical sake, but feel free to ignore.]

Thanks for the feedback. This is an interesting case. I'm not surprised by the existing behavior. merge: true basically says to merge in all the primitive values in the provided object, and your object doesn't have any primitive values, so it's a no-op. To accomplish your end goal, you could do:

// Use an explicit field mask that says to overwrite at the level of 'field'
ref.set({ field: {} }, { mergeFields: ['field'] });
// Or explicitly delete the nested field you don't want:
ref.set({ field: { nested: firebase.firestore.FieldValue.delete() } }, { merge: true });

That said, one could argue that the existing merge behavior for { } is never useful (since it doesn't do anything) and we should instead treat empty objects as primitive values for the purpose of considering what needs to be overwritten by the merge.

But changing this would be a breaking change, and I'm also a little worried it could lead to accidental data loss, e.g.:

let updates = { };
if (abc_changed()) {
  updates.abc = new_abc();
}
if (xyz_changed()) {
  updates.xyz = new_xyz();
}
...
ref.set(updates, { merge: true });

In the case that nothing changed, this would end up doing:

ref.set({ }, { merge: true });

which would end up deleting all fields if we were to treat { } as a primitive value to update.

So I think I'm inclined to leave the behavior as-is. What do you think? Maybe my original workarounds are sufficient for your situation?

[mchyb]

I expect the {} not to be treated like a primitive for the same reason you mentioned. In my example I'm updating nested "field" and not the whole document. Unexpectedly In this case {} is treated like a primitive type and it replaces previous value. On the contrary updating whole document with {} works fine and the document remains unchanged.

I believe my comment was a little imprecise so I created additional Stackblitz example.

[mikelehen]

Oh! I'm sorry. I read your report backwards (possibly because I was expecting the same behavior as you). It looks like we've gone back-forth on this behavior (#1168). I'll bring it up with the team and try to figure out what we really want to do going forward.

[mikelehen]

We discussed this internally and we think it's a bit of a toss-up. As mentioned, we used to have the old behavior but flipped to the current behavior (#1168). I think, like you, I actually slightly prefer the old behavior, but rather than flip again (and suffer the resulting churn / potential breakage), we are going to keep this as-is for now. We may reevaluate if we get more feedback.

Note that in the case that you know the document exists, you can just use update for this case:

ref.update({ field: {} });

Sorry for the inconvenience. 😦

[mohshraim]

i think this is the correct behaviour!!
Case 1168 about removing the inner object totally when it is empty object... and i think its not related to this case...

according to documentation you can use inner nested object

ref.set({ "field.color": "Red" } , { merge: true })

if you want to delete single inner field use firebase.firestore.FieldValue.delete()
if you want to update the whole inner field ref.set({ field: {} }, { merge: true });