Error with Firestore db persistence on Vue3

Shanzid01 commented 2 years ago

Describe your environment

Operating System version: MacOS 12.2.1 (M1 chip)
Browser version: Chrome 99.0.4844.74
Firebase SDK version: 9.6.4
Firebase Product: firestore
NodeJS: v16.14.0

Problem

Adding firestore persistence to Vue3 web app (followed instructions here) results in console errors when setting up snapshot listeners in firestore:

[2022-03-19T13:02:21.945Z]  @firebase/firestore: Firestore (9.6.4): INTERNAL UNHANDLED ERROR:  Error: Failed to execute 'put' on 'IDBObjectStore': #<Object> could not be cloned.
    at ti.put (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5969:24)
    at Proxy.ae (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:7166:22)
    at Proxy.updateTargetData (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:7133:21)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:9146:30)
    at Map.forEach (<anonymous>)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:9105:25)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:8508:20)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5659:49)
    at zs.wrapUserFunction (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5652:23)
    at zs.wrapSuccess (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5659:25)

index.esm2017.js?0829:5883 Uncaught (in promise) DOMException: Failed to execute 'put' on 'IDBObjectStore': #<Object> could not be cloned.
    at ti.put (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5969:24)
    at Proxy.ae (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:7166:22)
    at Proxy.updateTargetData (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:7133:21)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:9146:30)
    at Map.forEach (<anonymous>)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:9105:25)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:8508:20)
    at eval (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5659:49)
    at zs.wrapUserFunction (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5652:23)
    at zs.wrapSuccess (webpack-internal:///./node_modules/@firebase/firestore/dist/index.esm2017.js:5659:25)

Steps to reproduce:

initialize a basic Vue3 TypeScript application with vue-cli (instructions)
install firebase
set up firestore db persistence with enableIndexedDbPersistence
create snapshot listeners

Relevant Code:

Setting firestore persistence (this works fine):

const db = initializeFirestore(this.app, {
  cacheSizeBytes: CACHE_SIZE_UNLIMITED,
});
await enableIndexedDbPersistence(db);

Setting up snapshot listener (this throws error at runtime):

const q = query(
  collection(firebase.db, "albums"),
  where("ownerId", "==", userModule.user.uid)
);

onSnapshot(q, async (snap) => {
  const { docs } = snap;
  ....
});

I've tried clearing all application data in browser but the problem persists. Happy to provide additional logs/details about my implementation.

ehsannas commented 2 years ago

Thanks for reporting @Shanzid01 . I'll take a look.

ehsannas commented 2 years ago

Hi @Shanzid01 Would you be able to provide a repo that we can clone to reproduce the issue?

yoannes commented 2 years ago

I have the same issue, the only way to get it to work is disabling enableIndexedDbPersistence

ehsannas commented 2 years ago

@yoannes Would you be able to provide a minimal code repo that we can clone to reproduce the issue?

Shanzid01 commented 2 years ago

@ehsannas Here you go: https://github.com/Shanzid01/firestore-db-persistence-error

I've also deployed the code here: https://firestore-db-persistence-error.pages.dev So you should be able to see the error in the browser logs as soon as the page loads.

Edit: Added you as a collaborator on the repo. Feel free to push commits to the repo and test live on the Cloudflare Pages environment.

Shanzid01 commented 2 years ago

hello @ehsannas, were you able to access the repo?

ehsannas commented 2 years ago

Thanks a lot for creating this @Shanzid01 . Yes, I'm able to access it.

The issue seems to be specific to Vue and how it interacts with IndexedDb (local caching). While it may take a bit of time to pin down and fix this, you can avoid enableIndexedDbPersistence as a temporary workaround.

Shanzid01 commented 2 years ago

@ehsannas is there a workaround to still enable caching so that accessing the same firestore data every time doesn't cause a new read (i.e. doesn't cause a billable event)? And any approximate timeline on when this may be patched?

For context, we're running an app where users are accessing large volumes of the same (read-only) data many times from the same device, and we're concerned that this may become an issue for us in terms of the firestore bill that comes through.

ehsannas commented 2 years ago

Hi @Shanzid01 , unfortunately I don't think there's a workaround which keeps the caching enabled. I understand this can affect the amount of your reads. But since this seems to be a Vue-specific issue, we'll need some time for prioritization against other work and investigation and can't provide a specific timeline.

ehsannas commented 2 years ago

Would you be able to check whether you get the same error on newest Firebase SDK version?

Shanzid01 commented 2 years ago

Thank you @ehsannas, I really appreciate your transparency. I'll keep an eye out for potential fixes/workarounds.

As for the versioning, the project I shared with you previously is on SDK v9.6.10 (which I believe is the latest?) and it still throws the error. I'm not sure if there's a beta/canary that I can install, though.

Shanzid01 commented 2 years ago

Hi @ehsannas. I was able to diagnose and fix this issue in PR#6173. I've also updated the demo repository to reflect these changes and show that the patch works [see live demo].

Sidenote: This definitely isn't the most elegant work, but this is the best I could do given my limited knowledge of the firestore codebase. Even if the PR isn't merged, I hope others who face this issue can use this workaround to at least temporarily address this.

ehsannas commented 2 years ago

@Shanzid01 That's awesome! Thanks for you contribution to the codebase 🎉 Will review and comment on the PR.

typefox09 commented 2 years ago

Hi there, has this issue been fixed? I noticed the https://github.com/firebase/firebase-js-sdk/pull/6173 fix was not applied. We're having the exact same issue, however ours only appears when we try to access paginated data with enableIndexedDbPersistence

Code that's failing:

if (entry[0].isIntersecting && this.lastDoc && this.messages.length >= 20) { const messageRef = query(collection(db, 'conversations', this.conversationId, 'messages'), orderBy('created', 'desc'), startAfter(this.lastDoc), limit(20)) try { const newMessages = await getDocs(messageRef) this.lastDoc = newMessages.docs[newMessages.docs.length-1] } catch (erorr) { console.log('here in error') console.log(erorr) }

This fails with the same error mentions above on the getDocs call.

Environment is the same, Vue3 with the Firebase JS SDK version 9.

ehsannas commented 2 years ago

Hi @typefox09 , unfortunately this issue is currently not prioritized. Due to the concerns that were raised in #6173 we haven't merged this change. It seems that a function is being added outside of our codebase and only applies to Vue. The proposed fix will cause a performance hit.

Zohenn commented 2 years ago

While I cannot reproduce the issue with your repo, I might know the fix after looking at discussion in your PR. In order to prevent Vue from wrapping variables with Proxy (which enables reactivity), you can use markRaw. The code would look like this:

initFirestore(): Promise<void> {
  /** NOTE: This entire function runs without any errors! */

  this.db = markRaw(initializeFirestore(this.firebaseApp, {
    cacheSizeBytes: CACHE_SIZE_UNLIMITED,
  }));
  return enableIndexedDbPersistence(this.db);
}

Use markRaw if you want to store Firestore instances, document or collection references, etc. in component data, Vuex/Pinia module state. Docs: https://vuejs.org/api/reactivity-advanced.html#markraw

Zohenn commented 2 years ago

@typefox09

Also as for using markRaw(), from looking at the documentation, this is available on the Composition API, we are using the Options API.

Not sure why it's put at Composition API section, it works in any place Vue enables reactivity, that's why I mentioned Vuex/Pinia. I'm not a huge fan of Composition API myself, I mostly use it as a replacement for mixins.

Still, I would consider wrapping Firestore instances or references with markRaw a good practice, might help with performance and reduce possibility of similar bugs, and you don't need reactivity on those anyway. Just leaving it here for other folks passing by.

Zohenn commented 2 years ago

As I mentioned, you need to put your instance/reference in component data or Vuex/Pinia state, that's when Vue will make it reactive and that's what @Shanzid01 was doing. If you were to log this.db in reproduction repo with and without markRaw, you would see a difference.

Shanzid01 commented 2 years ago

For anyone coming across this and looking for at least a temporary fix, you can try using the patch-package library to create a fix for this. I've done so in my demo repository. I've been using this change in production for over five months and haven't seen any side effects.

The proposed fix will cause a performance hit.

That's true, however, the disadvantages of simply not using the cached DB were too significant to ignore, especially considering how insignificant the performance hit associated with this patch actually is in my project.

twis4t commented 1 year ago

Use markRaw if you want to store Firestore instances, document or collection references, etc. in component data, Vuex/Pinia module state. Docs: https://vuejs.org/api/reactivity-advanced.html#markraw

Thanks! this solved the problem

typefox09 commented 1 year ago

This worked for me also, thanks. Problem came from storing the last document in data() for pagination. Adding markRaw to that solved the issue.

patrickcorrigan commented 9 months ago

Just came across this issue myself. This thread was really helpful.

In my case it was caused by one of my queries was being executed with a proxy. Just need to call toRaw() on this.

Setting firestore logging to debug allowed me to see the query.

It seemed to only be an issue when there was also a snapshot listener on the same collection.

It was also interesting that if this document had been queried before but without the vueRef it worked fine. I'm guessing the query is stored with the cached data.

      const doc = (
        await db
          .collection("blah")
          .where("game", "==", toRaw(this.vue3Ref))
          .get()
      ).docs[0];

koenbetsens commented 1 month ago

toRaw() saved my day. Thank you!

firebase / firebase-js-sdk