mesqueeb / vuex-easy-firestore

Easy coupling of firestore and a vuex module. 2-way sync with 0 boilerplate!
https://mesqueeb.github.io/vuex-easy-firestore
MIT License
234 stars 28 forks source link

Sequential use of arrayUnion and arrayRemove over the same array do not work #217

Open artfabrique opened 5 years ago

artfabrique commented 5 years ago

How to reproduce

Try to perform this sequential dispatches;

var newElement = { test: "foo" };

context
  .dispatch("patch", {
    id: "myID",
    myArr: arrayRemove(oldElement)
  })
  .then(result => {
    console.log("element removed! ", result);
    return context.dispatch("patch", {
      id: "myID",
      myArr: arrayUnion(newElement)
    });
  })
  .then(result => {
    console.log("element added! ", result);
  })
  .catch(error => {
    console.log(error);
  });

As a result, we will have a single batched update from V.E.F. and only first used method will be put into _methodName prop of the update. In this particular case the result of batched V.E.F. update will be:

updates: [
        myArr: {
            _elements: [
                {test:"bar"}, // <--
                {test:"foo"} //  <-- both elements present
            ]
            _methodName: "FieldValue.arrayRemove" // <--here is the problem
            __proto__: FieldValueImpl
        },
        id: "myId",
        updated_at: "Wed Jun 05 2019 03:33:29 GMT+0300 (Moscow Standard Time) {}",
        updated_by: "ZSCsxLxNpqP4ruCReTHGmwWgQks1"
    ]

If I offset the second dispatch in time withsetTimemout(5000) and it falls into the next batch all works as expected.

artfabrique commented 5 years ago

I'll try to break batches via using dispatch("batchSync") between two array operations. Supposedly it should send all accumulated updates to the firestore.

artfabrique commented 5 years ago

any updates on that issue?

fega commented 5 years ago

Having a similar Issue here

mesqueeb commented 5 years ago

So after having looked into this, apparently the way I was merging two arrayUnions on a patch call has broke because of the way the Firebase SDK works changed slightly.

My original merge code:

https://github.com/mesqueeb/vuex-easy-firestore/blob/master/src/module/actions.ts#L106

Workaround:

While I work on the solution, for now please dispatch('batchSync') in between any arrayUnion or arrayRemove calls.

Solution:

I'll change the library to queue arrayUnions and arrayRemoves in the shape of my own class wrappers, then only once the queue finished and the changes are pushed to Firebase I will convert the class wrapper to the actual Firebase FieldValue. This way I can merge multiple calls correctly without the danger of it breaking again in the future.

mesqueeb commented 5 years ago

Hi. I have further looked into this. I couldn't solve this properly since the Firebase SDK doesn't allow to send a payload with both ArrayRemove and ArrayUnion. Simply because the data object with the updates would have a duplicate key... eg.

{
  id: "myID",
  myArr: arrayUnion(newElement),
  myArr: arrayRemove(otherElement),
}

I couldn't find a way to merge both of these updates into 1 batch...

For now: stick to doing dispatch('batchSync') in between calls.

In the future: I'm thinking of making vuex updates immediately and spreading out firestore sync over multiple batches automatically whenever there's a conflict between both arrayUnion and ArrayRemove.

Will update here later.


After about two years of open source, I finally got accepted for Github Sponsors!

💜 github.com/sponsors/mesqueeb 💜

A little about me:

If anyone was helped with vuex-easy-firestore, I'd greatly appreciate any support!

BTW, donations get's paid DOUBLE by GitHub! (they're alchemists... 🦾)

Going forward 👨🏼‍💻