use arrayUnion on non-array values

[mesqueeb]

Currently using arrayUnion on a prop that's not an array will not work. I will change this behaviour so it will change the value into an array for you.

---

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

💜 github.com/sponsors/mesqueeb 💜

A little about me:

• I love open-source
• 6 months ago I got a son, and am trying to be an awesome father 😅
• I'm doing freelance work here and there as my main job

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 👨🏼‍💻

• I got great plans for the future of vuex-easy-firestore going above and beyond!! Look forward to it!!
• On to many more years of open-sourcing! 🎉

[zabaat]

Would this be for the best? maybe it should throw an error to the console when you are in debug mode or something rather than turning it straight into an array?

I'm not adverse to the change you suggest really, but thought I'd throw that out there.

[mesqueeb]

I for one would love to use arrayUnion a lot but, i'm often in a situation where I don't know if a doc will be a new doc or an existing doc that has to be patched. That's why I implemented "set" in Vuex easy fire. But it's really annoying for me to always have to check if the doc and prop exists and is an array in order to be able to use arrayUnion. ><

But I do see your point as well. Haha

_{Sent with GitHawk}

[mesqueeb]

@zabaat

How about this: dispatch('user/set', {prop: arrayUnion('hi')})

• if the prop is undefined (inexistent)
  • it will creates a new array with the value of arrayUnion prop: ['hi']
• if the prop exists but is not an array
  • vuex-easy-firestore throws an error and abort the mutation and sync

[louisameline]

That's actually how Firestore works. If the initial property value is anything but an array (including undefined), it overwrites the property value with an array that contains the value(s) provided. If it already was an array, the new values are added to it if they were not already present.

So yes, dispatch('user/set', { prop: arrayUnion('hi') }) should definitely create (or update existing) an array in all cases. Now, about an error being thrown:

if the prop is undefined [...], if the prop exists [...]

There is a wrong assumption here: you think you know the property before the update. Until this library implements transactions, you actually don't know what the property value will be when the write operation is carried out.

So we probably shouldn't let users rely on an Error they think they could catch when they mess up their types, because if this happens at Firestore, they will actually never get one (again, unless in a transaction). I'd rather log a big warning, but not promise an Error you're not going to be able to provide consistently.

This wrong assumption also seems to be made in the current way set in collection mode operates, as it checks if a doc exists locally to decide to use insert or patch. Two scenarios come to mind:

• the document may have been created after the last time he fetched the collection and we don't know (not everyone creates new documents with a random id)
• the user may want to update a document he has willingly not retrieved

I might very well be wrong as I currently have a hard time following what really happens in the code, but I fear there could be wrong methods being called there, resulting in potential data loss. We need to make a list of all potential scenarios at some point to make sure we have everything covered