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

updated_at en updated_by guards, only in some cases #305

Open Fulbert opened 4 years ago

Fulbert commented 4 years ago

Hello,

Thanks for this awesome tool!

I have a guarded property on my store, selected, which is used only by the UI, and I don't want to sync on Firestore. If the guards works well, the document is still synced everytime I change the selected value, because of the updated_at and updated_by value. Of course, I can add the two properties to the guards too, and it works, but I actually still want to use them for some other cases.

Did I miss something that could help, or would I need to use other property name and add logic to save updates when needed?

Thanks!

mesqueeb commented 4 years ago

Hello. Thanks for your issue! This is indeed an issue. In version 2.0 this issue is already non-existent, but in v1.0 we'd need to make some modifications for this edge case.

If you're interested in contributing, I can point you to the area that needs to be updated. 😉

Otherwise, you can work around it by writing a custom wrapper function for the patch action, that checks if only the selected prop is edited or not, and if only selected was changed not to dispatch the patch action.

Fulbert commented 4 years ago

Well, I guess this is somewhere there : https://github.com/mesqueeb/vuex-easy-firestore/blob/017ee80e4a638019819443938c1ff96d0a920681/src/module/getters.ts#L174

I need to know if this prop delete is because of a guard... I'll take a look.

Fulbert commented 4 years ago

@mesqueeb I just come back to this one, because it actually cause bug in my app... Because even with guards on updated and created, it still try to patch (nothing)... Unfortunately because of firestore rules, some users can't update...

Because my app is not yet in prod, I can open the rules a little bit to fix it. However if you can tell where to fix this, I would love to.

Thanks 👍

mesqueeb commented 4 years ago

@Fulbert there is actually an easy workaround I want to suggest you use instead:

https://mesqueeb.github.io/vuex-easy-firestore/hooks.html#hooks-after-local-changes-before-sync

{
  // in your module config
  sync: {
    patchHookBeforeSync: function (updateFirestore, doc, store) {
      // check if you are only trying to update the "selected" prop or not, if so, return early
      if (selected in doc && Object.keys(doc) === 1) return
      // else, continue:
      updateFirestore(doc)
    },
  }
}

Can you try this out and let me know how it goes? The exact if sentence you write might need some tweaking depending on your setup.

Fulbert commented 4 years ago

I actually fixed it like this yesterday, few minutes before I thought that I can mutate my selected prop with my own mutation... Since the beginning.

Sorry for that 😳

louisameline commented 4 years ago

We'll definitely make requests to Firestore only if they are non-empty in the future, so this will be solved.

In the meantime, I have a question for you @Fulbert : is there a reason why your guarded property is within state.data (if data is the prefix you chose) instead of at the root of state, in which case it would not be synced anyway (no need for a guard)? I'm trying to get a glimpse of the use cases where people use guarded properties. Thank you

Fulbert commented 4 years ago

I have the following collections :

In my app I have list view for all this collections and I can select multiple items.

Instead of having locallyan array of selecteds IDs, I prefer to have a property "selected" in each document. But I don't want to sync it because of course each user can select their own documents.

User can trigger actions on multiple items, thanks to selection.

However I was using the vuex-easy-firestore patch action to mutate my "selected" property (alone) but then I realized I should actually use my own action to avoid syncing with firestore (as described in my previous post).

I still think it shouldn't sync if there's nothing to mutate, but in my case I'm still not sure what's the best way to do it.

You can see a bit more here : https://github.com/ServiceTaskManager/servicetask/blob/ad55133e4350255e6ae3cd5c9d72a2d89ffec97f/src/store/firestore/index.js#L78 And here https://github.com/ServiceTaskManager/servicetask/blob/master/src/components/generic/StItem.vue

Le ven. 15 mai 2020 à 17:41, Louis Ameline notifications@github.com a écrit :

We'll definitely make requests to Firestore only if they are non-empty in the future, so this will be solved.

In the meantime, I have a question for you @Fulbert https://github.com/Fulbert : is there a reason why your guarded property is within state.data (if data is the prefix you chose) instead of at the root of state, which is not synced? I'm trying to get a glimpse of the use cases where people guard properties. Thank you

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mesqueeb/vuex-easy-firestore/issues/305#issuecomment-629321962, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGCSIF4TXJ3V2PMVAKD4YTRRVPBJANCNFSM4LQZ674A .

louisameline commented 4 years ago

Oh great thank you, I understand now.

It's ok to use a non-vef action or mutation to mutate your property until we fix. I'll check tomorrow if a quick patch can be done for this issue.

But in all cases, there will be another preferred way to handle your local properties of documents loaded by a collection in the future.