Feat: update existing document

[spoxies]

There are some (closed) issues/questions about updating the custom 'd.' values using geoFirestore.set(). I'd like to share some of my findings, because it might save the dev's a bit of work.

Please note that I'm no JS expert...and I'm also not good at writing short and to the point

TL DR; There is no official update() function just yet, but I wrote a suggestion as inspiration that I will commit in my fork. I will not insult anyone by doing a pull/merge request, because goto: 4;.

Also @MichaelSolati said;

...in version 3.0.0 the set and update will align with how the Firestore SDK works, where you can send options for merging and what not.

The cases; [#40] If you update only the "coordinates" on an existing document using set(), the "d." custom-fields get removed.

As @MichaelSolati said;

The set function requires a full document, so the original with modifications,

However the batch update from geoFirestore actually already uses a merge: batch.set(ref, [...], { merge: true }); so that suggests a possible work around would be to use:

geofirestore.set({ 'key': {coordinates:[..]} } )

instead of:

geofirestore.set('key', {coordinates:[..] } )

...that might work if you only update the coordinates, but even if so we would not be out of trouble yet because:

[ #45] Updating a single d.field with firestore.{set() || update()} removes other keys

If our document has multiple custom fields (d.) and you only want to update one nested d. field, then even the update() function of firestore itself might not work as expected.

db before: { l: '[loc]', g: 'loc_hash' d : { keyToRemain : 'staying alive', otherKey : 'leave me alone', keyToChange : 'total-make-over' } }

update: let updateObj = {d : {keyToChange : 'new-face' }; firestore.col(ref).update(uid, updateObj);

db after: { l: '[loc]', g: 'loc_hash' d : { keyToChange : 'new-face' } }

However apparently if you use a source path within update() like so ['d.keyToChange'], a merge does happen.

update: let updateObj['d.keyToChange'] = 'new-face'; firestore.col(ref).update(uid, updateObj); db after: { l: '[loc]', g: 'loc_hash' d : { keyToRemain : 'staying alive', otherKey : 'leave me alone', keyToChange : 'total-make-over' } }

This 'trick' works with update() and not with, .set(ref, [...], { merge: true }); because set() needs document formed data. source.

The commit/suggestion below adds an update() method that:

will not remove the custom d. fields geoFirestore.update('existing_key', { coordinates: geoPoint});

will append custom keys to d. geoFirestore.update('existing_key', { coordinates: geoPoint, keyToChange : 'new-face'});

[MichaelSolati]

@spoxies I invite you to open a PR, while what you bring up will be resolved in version 3, version 3 is kind of a different beast. Some users may not want to upgrade, so having this functionality in version 2 would be nice.

I'd just ask that you update the docs and include tests to validate it.

[spoxies]

@MichaelSolati Thank your kind response and invitation.

I will update the docs, write some more tests to validate and open a PR as soon as possible.

[len-art]

I am super interested in having this feature as well and i appreciate this contribution very much! Was thinking about forking it and just hacking it together by myself. Looking forward to the PR being created and merged :)

[spoxies]

TL DR;

• With https://github.com/spoxies/geofirestore-js/commit/5ca41c51c699ea5a765b39b217666913ae94405a , it would still require to pass {coordinates: ..} to update()
• With PR [#53] passing {coordinates: ..} has become optional for update()

What: Passing {coordinates: ..} to update(), has become optional. I think it does make sense that update(), updates and therefore does not require {coordinates: ..}. But this might be a big decision to make and something worth looking at/thinking about

Why : In my opinion the strong point of geofirestore is; that is allows for essential de-normalized data. And therefore allows for fast operations and scalability as is advised. In this scenario just updating/adding a geoFirestore d. property and not the location, is common.

What document one wishes to update does not have to be dependent on its (current) geolocation perse. So with the PR it is possible to do: geoFirestore.update('car_uid', {color : 'red' }); whilst maintaining some abstraction/control by the library https://github.com/geofirestore/geofirestore-js/issues/45#issuecomment-437086502

How: Making {coordinates :..} optional is wrote more as addition, that could be removed. So I haven't optimised the if/else flow of the lib itself (yet) and there is some overlapping. Besides this there is plenty of room for improving (especially the tests and formatting), but I hope it is a good start.