feat(useIDBKeyval): new integration - Idb-keyval wrapper #2335
Conversation
Harmony222
changed the title
There was a problem hiding this comment.
Needs tests still. I'm happy to write them, but can I get some 👀 on the signature? Specifically I'm curious about the shallow option vs having the user pass in the ref/shallowRef/computed themselves.
Actually (@Harmony222 says) it seems IndexDB isn't defined for Vitest's environment. Do we want to polyfill IndexDB in Vitest core @antfu?
| * | ||
| * @default false | ||
| */ | ||
| shallow?: boolean |
There was a problem hiding this comment.
Can we remove this option and the associated code by having the user pass in a computed, ref, or shallowRef?
There was a problem hiding this comment.
I was following the same pattern as useStorage and useStorageAsync with the shallowRef here, but can change that if needed.
There was a problem hiding this comment.
I think we could reuse useStorage and implement StorageLike to avoid repeating code. Just like useLocalStorage and useSessionStorage.
| @@ -151,6 +156,7 @@ | |||
| "drauu": "^0.3.2", | |||
| "focus-trap": "^7.0.0", | |||
| "fuse.js": "^6.6.2", | |||
| "idb-keyval": "^6.2.0", | |||
There was a problem hiding this comment.
It also requires to be added to peer deps
There was a problem hiding this comment.
added to peer deps
There was a problem hiding this comment.
@sxzz Regarding the first comment, thank you for your patience as I looked into this.
I looked through useLocalStorage and useSessionStorage to understand how they reuse useStorage. I believe you are suggesting that we could use indexDB storage in place of window?.localStorage, the problem I see is that useStorage converts all values to strings, and a big reason our team wanted to use indexDB is so we can store values other than just strings.
I also took a closer look at StorageLike, its get and set functions get and return strings, so I am not sure that using indexDB in this way would provide any gains to using localStorage as is.
Please let me know if I am misunderstanding something.
|
@JessicaSachs I think we could mock functions of ping @antfu Is this function necessary and acceptable? |
|
By the way, the keyword |
adding first test and fake indexeddb
| if (data.value == null) | ||
| await del(key) | ||
| else | ||
| await update(key, () => ({ ...data.value })) |
There was a problem hiding this comment.
Question, did this have the same issue that you were messaging me about? The issue with ...someString etc?
There was a problem hiding this comment.
yeah looks like that one is having an issue too. This is what jimmy and I added when we worked through the bug the other day, it had something to do with proxy objects not able to be stored in index db if I remember correctly.
There was a problem hiding this comment.
changed this logic to only make copy with spread syntax if data.value is an object or array
|
hi @sxzz the contributor generation script is failing |
|
There are some TS type errors |
|
Thank you @sxzz, I have addressed the TS type error. |
|
@sxzz |
|
I am fine to have this as-is for now. Later we could see how we can refactor this with |
Screen.Recording.2022-10-19.at.9.17.51.AM.mov
Description
Provides a composition wrapper around idb-keyval, which is a key value store api on top of Indexed DB. Gives you typed keys and values without the need of a serializer, adds web worker support, and lets you use more storage space when compared to using the local storage variant.
Additional context
This partially addresses issue #230, but does not add full support for indexed DB (just for typed keys and values).
closes #230
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).