Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetchAndAdd returns always cached data for dynamic firestorePath #262

Open
RolandCsibrei opened this issue Nov 16, 2019 · 9 comments
Open

Comments

@RolandCsibrei
Copy link

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]

const identifier = createFetchIdentifier({where, orderBy})

Can be?

        const identifier = createFetchIdentifier({where, orderBy, pathVariables})
mesqueeb added a commit that referenced this issue Nov 16, 2019
@mesqueeb
Copy link
Owner

@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
Copy link
Author

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
Copy link
Author

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

@mesqueeb
Copy link
Owner

@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?

@mesqueeb mesqueeb reopened this Nov 17, 2019
@RolandCsibrei
Copy link
Author

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
Copy link
Owner

@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
Copy link
Owner


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! 🎉

@louisameline
Copy link
Collaborator

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
Copy link
Collaborator

Using a single module relates to #164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants