Skip to content

Don't drop empty object during merge #1851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 19, 2018
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

We currently drop empty objects when during set({merge: true}) since we only add leaf nodes to the field mask. We need to consider an empty object a leaf node.

Port of firebase/firebase-js-sdk#1202

@schmidt-sebastian schmidt-sebastian merged commit 08dede0 into master Sep 19, 2018
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt-emptymerge branch September 19, 2018 21:52
@willtr101
Copy link

willtr101 commented Oct 2, 2018

This caused a serious issue for us. Assuming we already have document.items, where items is a dictionary contains existing keys/values.
Now, if we update that document, with items set to an empty object {}, merge flag is true, existing items will be dropped, returning a new empty document.items, while we expect update/set with merging flag is true should keep the existing data, not wiping them {object }+ {} = {object}, not empty. Please reconsider this PR. It seems this PR cared on object.subobject where subobject is empty, keeps them as a node, however, this will fail when updating a root object.

@schmidt-sebastian
Copy link
Contributor Author

We will discuss your comment internally. Note that you can make the behavior of set() explicit by providing the list of field paths you wish to merge. This will override any setting on our end.

@firebase firebase locked and limited conversation to collaborators Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants