upsert$ and update$ both use set under the hood

[santekotturi]

My app was able to insert some partials as documents in part because this plugin uses firebase's set instead of update. I would like to call firebase.update which will fail if the doc id that is trying to be updated doesnt exist. Instead this doc will either overwrite or create a new doc (unless I read the src code wrong).

Can we implement a true update instead of set, {merge: boolean}?

This SO post denotes the diff: https://stackoverflow.com/questions/46597327/difference-between-set-with-merge-true-and-update

I tried writing assertions in the toFirestore converter but this accepts partials just fine which I don't fully understand how because according to the src code, both update$ and upsert$ both call set which requires the full doc (not partials).

[joaqcid]

hi, when we wrote the NgxsFirestore helper we didnt implement firestore's update in order to keep it simple. if you need a different implementation, you can write your own version of service or implement the update method in the NgxsFirestore update method

[santekotturi]

What complexity would this add? Would updates' not being completed require a bunch of extra handling in this plugin? I'm mostly curious if there's something inherently difficult about this task that you're aware of?

Do you use a workaround in your own apps for checking to make sure the doc exists before you update it? Some sanitization/assertions before performing writes? I'm open to other ideas

[joaqcid]

well, i havent found myself needing to validate a doc existed prior to update it.

what i've done in some cases is checking if the payload im about to pass to update$ contains the id field. Since my app only allows you to edit docs where the id is present, i can assume that the doc exists in the db. But its not restrictive as firestore's native update.

if you need to be restrictive, i would implement a new method in your service, something like:

export class YourFirestore extends NgxsFirestore<YourModel> {

  updateIfExists(data){
    return this.adapter.firestore.doc(`${this.path}/${id}`).ref.withConverter(this.converter).update(data);
  }

}

[santekotturi]

Thanks, that helps us a lot. Using updateIfExists indicates that there's errors upstream in my app if a document ID is being lost but it will be useful for storing rapidly updating data like read_receipts on messages based on viewport scroll position.

[santekotturi]

@joaqcid not sure what changed but this stopped working, specifically ref.withConverter(this.converter).update(data) doesn't actually implement the converter.

I can't tell from the firebase doc if withConverter is intended to work with update() or only with other operations like set()

I was on 0.1.29 of this library. I updated from 0.1.30 to 0.1.31 to see if they patched this (it used to work)

Can you check to see if this wrapper does work on your end still:

  updateIfExists(data){
    return this.adapter.firestore.doc(`${this.path}/${id}`).ref.withConverter(this.converter).update(data);
  }

[joaqcid]

hi @skotturi did this stop working on 0.1.30? because there wasnt really any change between 29 and 30...

[joaqcid]

hi @skotturi

i implemented updatedIfExists on my repo and indeed its not working, but its not working on current version nor 0.1.29...

as you mention it only works with set method

[santekotturi]

Unfortunately I cannot find too much in the docs. They only mention needing to define your type using Partial<U> when used with set() because most of their examples use it with .collection()

I guess it's not that hard to call it manually 😅

   updateIfExists(id: string, data: Partial<Village>) {

    const convertedData = this.converter.toFirestore(data)

    return of(
      this.adapter.firestore
        .doc(`${this.path}/${id}`)
        .ref
        .update(convertedData)
    );
  }

[joaqcid]

curious to know when it stopped working since i rollback to prior version on the lib and angular/fire and firebase but i never got it to work...

[santekotturi]

I'm leaning towards thinking that it wasn't actually working before, it was just silently passing over the converter (I started to notice un-encoded data a while back but didnt think twice about it until now) but calling update() instead of set() addressed my main issue of preventing writes when a docId was missing