-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: add IndexedDB Store implementation #120
Conversation
Deploying with Cloudflare Pages
|
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.
Looks ok to me, proposed some changes. I'm not super familiar with the code so might be worth getting some feedback from hugo as well.
* @param {number} [dbVersion] | ||
*/ | ||
constructor(dbName, dbVersion) { | ||
this.#dbName = dbName |
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.
Are these widely enough available for us to use ? If I recall correctly they were considered in js-multiformats but then dropped because adoption was not there yet.
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 can put underscores...
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.
/** @type {import('p-defer').DeferredPromise<boolean>} */ | ||
const { resolve, reject, promise } = defer() | ||
|
||
const getReq = store.get(DATA_ID) |
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.
Nit: I find it handy to have this function around when working with IDB
/**
* @template T
* @param {IDBRequest<T>} request
* @returns {Promise<T>}
*/
const request = (request) => new Promise((succeed, fail) => {
request.onsuccess = () => succeed(request.result)
request.onerror = () => fail(request.error)
})
With which you could simply change above line as follows
const getReq = store.get(DATA_ID) | |
const result = await request(store.get(DATA_ID)) |
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.
LGTM
An IndexedDB store implementation that uses RSA keys for the browser.
An IndexedDB store implementation that uses RSA keys for the browser.