Skip to content

Idb#1601

Open
paultranvan wants to merge 7 commits intomasterfrom
idb
Open

Idb#1601
paultranvan wants to merge 7 commits intomasterfrom
idb

Conversation

@paultranvan
Copy link
Copy Markdown
Contributor

@paultranvan paultranvan commented Apr 11, 2025

This is the equivalent of #1598 , but for IndexedDB, so this time for browser.

I will complete with performances measure later

@@ -1,2 +1,3 @@
export { default } from './CozyPouchLink'
export { default as SQLiteQuery } from './db/sqlite/sqliteDb'
export { default as IndexedDBQueryEngine } from './db/idb/idb'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming is not homogenous with SQLiteQuery (you added the Engine suffix)

Comment on lines +29 to +31
const request = version
? indexedDB.open(fullDbName, version)
: indexedDB.open(fullDbName)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version is used there, but we never provide it? Do we expect to use it in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is never provided, but it might be (and I tried few things during tests), so I prefer to keep it for now

}

async openDB(dbName, version = undefined, { forceName = false } = {}) {
return new Promise((resolve, reject) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ openDB() is not declared as async in dbInterfaces file, but more important, it is not awaited in PouchManager

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

return resp
} catch (err) {
logger.error(err)
return null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ the dbInterfaces files does not declare allDocs to return a nullable type

Copy link
Copy Markdown
Contributor Author

@paultranvan paultranvan Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I fixed the interface, it was already returning null in sqlite

limit,
partialFilter,
skip = 0,
shouldRecreateIndex = false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find any code that set shouldRecreateIndex. Is that a change that will be implemented later?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is helpful to quickly set this for debug purposes

@paultranvan paultranvan marked this pull request as ready for review April 24, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants