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

fetchAndAdd returns always cached data for dynamic firestorePath #262

Open RolandCsibrei opened 5 years ago

RolandCsibrei commented 5 years ago
export const code = {
  firestorePath: 'codelist/{list}/values',
  firestoreRefType: 'collection',
  moduleName: 'code',
  statePropName: 'codes',
  namespaced: true,
  state: {
  },
  actions: {
    async getCodes ({ commit, dispatch }, list) {
      dispatch('fetchAndAdd', { list: list })
    }
  }
}

Despite that list changes in getCodes, the result is still read from cache, because pathVariables are not taken into account when building the identifier:

        const identifier = createFetchIdentifier({where, orderBy})
        const fetched = state._sync.fetched[identifier]

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

Can be?

        const identifier = createFetchIdentifier({where, orderBy, pathVariables})
mesqueeb commented 5 years ago

@RolandCsibrei thanks for looking into this and also providing me with a solution. I made the change as per your suggestion in v1.34.5 Please try it and let me know. 😄

--
Vuex Easy Firestore was made with ♥ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee. ☕️
You can also reach out on twitter if you want a one-on-one coding review/lesson. 🦜

RolandCsibrei commented 5 years ago

Sadly this doesn't fix all the issues.

The following code snipet shows the problem with caching:

export const code = {
  firestorePath: 'codelist/{list}/values',
  firestoreRefType: 'collection',
  moduleName: 'code',
  statePropName: 'codes',
  namespaced: true,
  state: {
  },
  actions: {
    async getCodes ({ dispatch }) {
      dispatch('fetchAndAdd', { list: 'list1' })
      dispatch('fetchAndAdd', { list: 'list2' })
      dispatch('fetchAndAdd', { list: 'list3' })

// make some changes in the FB console and call getCodes again
// changes are not reflected, data is read from the cache
    }
  }
}

In some cases this behaviour is ok, however maybe an option should be added, like nocache, to force rereading of the data from the db?

Thank you!

RolandCsibrei commented 5 years ago

The cached data got crazy after calling fetchAndAdd a few times with changig dynamic pathVariable, maybe there is a problem with object references.

mesqueeb commented 5 years ago

@RolandCsibrei What do you mean with "data got crazy" ? Can you try and explain to me a little more about exactly what the problem is you're facing?

RolandCsibrei commented 5 years ago

Writing from my phone so I will be less verbose. If you call fetchaAndAdd with pathVariable1 it returns fresh data, no caching. Call fetchaAndAdd again with pathVariable2, returns fresh data, no caching. Now call fetchaAndAdd with the same pathVariable1, it hits the cache, returns data, but dbrefs seems to be bound to data defined by pathVariable2, and not pathVariable1.

mesqueeb commented 5 years ago

@RolandCsibrei I completely understand now. I'll try and create a failing test based on these steps, then I'll try to fix that test asap.

mesqueeb commented 4 years ago

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

louisameline commented 4 years ago

I am wondering if we should allow the change of path variables after instantiation. Common sense says to have 1 resource = 1 module, and that's what is considered so far for the next version of the library. I'm just wondering if someone will come up and say "I need to do it because I have 500 documents loaded at the same time and I can't afford to instantiate a module for each of them as it would consume too much memory". But is it likely? Destroying documents when we don't need them would let the garbage collector do its work. The data could even survive in the store after the destruction of the module, thus leaving no memory overhead either. What would be other arguments for keeping the possibility to change variables of the path? Any feedback appreciated!

louisameline commented 4 years ago

Using a single module relates to #164