-
Notifications
You must be signed in to change notification settings - Fork 991
[patch] Pre-process variables before adding into indexedDB #6173
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
base: main
Are you sure you want to change the base?
Conversation
Serialize and deserialize key-value pairs before adding them into indexedDB. This prevents invalid values from being added into indexedDB, and fixes issue firebase#6087
|
ehsannas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @Shanzid01 . It's very much appreciated.
We'll need to take some time to understand the underlying issue and whether there's a way to address it without affecting all transactions.
| try { | ||
| keyOrValue = | ||
| keyOrValue != null | ||
| ? JSON.parse(JSON.stringify(keyOrValue)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would mean we do this for every transaction we do. We need to think about implications (performance etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! You're correct, I'm concerned about the performance impact as well. And this definitely doesn't fix the underlying issue (functions being added into the indexedDB), it's only a band-aid for those who desperately need a solution.
The firebase team should definitely consider a better solution for the final patch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why a function is being written to indexed db or why it only happens to Vue, if you know the answers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't determine why functions were being added, and why that was happening only in Vue3. But I was able to see what functions/Objects were being added:

The culprit here seems to be the document->fields->Proxy Object. Firestore is attempting to insert this Proxy object into the IndexedDB and that's causing the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not added by our codebase (reference).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's very likely. Would also explain why this is only happening on Vue3. Possibly in one of the shared libraries for reactivity. Will investigate more and see if I can find something.
Serialize and deserialize key-value pairs before adding them into indexedDB. This prevents invalid values (like functions) from being added into indexedDB, and fixes Issue 6087.
You can test these changes live here [see code].