-
Notifications
You must be signed in to change notification settings - Fork 58
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
Feat: update existing document #52
Comments
@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. |
@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. |
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. |
TL DR;
What: Why : What How: |
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, becausegoto: 4;
.Also @MichaelSolati said;
The cases;
[#40] If you update only the "coordinates" on an existing document using
set()
, the "d." custom-fields get removed.As @MichaelSolati said;
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 offirestore
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 });
becauseset()
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'});
The text was updated successfully, but these errors were encountered: