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

Fix the support of non-primitives in ArrayUnion #331

Closed louisameline closed 4 years ago

louisameline commented 4 years ago

Currently, there is this code:

    ArrayUnion.prototype.executeOn = function (array) {
        this.payload.forEach(function (item) {
            // if array of object, find it by "id" (ex.: works with doc reference)
            var index = isAnyObject(item) && item.id !== undefined
                ? array.findIndex(function (_item) { return _item.id === item.id; })
                : array.indexOf(item);
            if (index === -1) {
                array.push(item);
            }
        });
        return array;
    };

I don't know what use case you had in mind when you wrote

if array of object, find it by "id" (ex.: works with doc reference)

I'm not sure why the fact that an object with the same id exists in the array would dismiss the add of the item if other properties are different?

But besides this strange id thing, this code doesn't work well for arrays of objects. Let's say my array currently is like this: [{ message: 'hello', author: 'Tom' }, { message: 'world', author: 'John' }]

The next time you make an ArrayUnion with { message: 'hello', author: 'Tom' } to the array, the item will be added a second time locally (the array will then have 3 items), while Firestore will recognize that the object is already there and will not add it (the array will stay with its 2 items).

The solution is, instead of

? array.findIndex(function (_item) { return _item.id === item.id; })

to write this:

// isEqual is taken from Lodash for example https://lodash.com/docs/4.17.15#isEqual
? array.findIndex(_item => isEqual(_item, item))

Same goes for ArrayRemove

mesqueeb commented 4 years ago

Yeah. need to do this. we could use isEqual of lodash-es which supports treeshaking and is side-effect free.

louisameline commented 4 years ago

If it's ok for you then, I'll make the PR today. It's like 2 lines of code, shouldn't be long :)

mesqueeb commented 4 years ago

fix included in latest version and also upgraded ava to latest version!