Page MenuHomePhabricator

[web-db] start persisting SQLite content
ClosedPublic

Authored by kamil on Mar 27 2023, 4:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 16, 11:06 AM
Unknown Object (File)
Sat, Jun 15, 9:54 PM
Unknown Object (File)
Fri, Jun 14, 8:06 AM
Unknown Object (File)
Thu, Jun 13, 12:53 PM
Unknown Object (File)
Thu, Jun 13, 8:12 AM
Unknown Object (File)
Wed, Jun 12, 11:32 AM
Unknown Object (File)
Tue, Jun 11, 7:23 AM
Unknown Object (File)
Tue, Jun 11, 5:41 AM
Subscribers

Details

Summary

Saving database file in indexed db

Depends on D7185

Test Plan
  1. Check in devtools if indexedDB key exists
  2. Reopen app and save something into db - database should be read from the indexed db, not freshly created

image.png (520×1 px, 95 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
web/database/worker/db-worker.js
218 ↗(On Diff #24167)

this is the line I am not sure how to handle:

  • we can only start persisting and resolve the promise (promise with app request) to let the app proceed and trust that worker will persist data (current one)
  • await this here to make sure it's persisted but this might slow down the main thread in the app
kamil published this revision for review.Mar 27 2023, 5:10 AM
tomek added inline comments.
web/database/worker/db-worker.js
141–144 ↗(On Diff #24167)

We don't have to export the data when the key is missing

218 ↗(On Diff #24167)

What is the downside of using the same approach for all the operations?

This revision is now accepted and ready to land.Mar 30 2023, 4:48 AM
web/database/worker/db-worker.js
218 ↗(On Diff #24167)

I meant a general difference between using persist(); vs await persist() and my concerns are applying to all operations

kamil edited the test plan for this revision. (Show Details)

add debounce

Adding debounce to persisting,
I set up the database for drafts and persist was not called on each character changed but was debounced as expected.

web/database/worker/db-worker.js
141–144 ↗(On Diff #24167)

in that order flow is complaining: Cannot call encryptDatabaseFile with encryptionKey bound to key because null or undefined [1] is incompatible with CryptoKey

web/database/utils/constants.js
8 ↗(On Diff #24468)

We can probably name it a little different so that it is obvious that it is a duration of debounce.

Throttling is a great improvement compared to just calling this function but it still might break when db save takes more than 300ms. We should probably handle that case differently.

web/database/worker/db-worker.js
203–224 ↗(On Diff #24549)

Maybe we can improve this code a bit by extracting common part after if? It seems like every branch results in persist and return.

This revision was automatically updated to reflect the committed changes.
web/database/worker/db-worker.js
203–224 ↗(On Diff #24549)

addressed in D7288