Closed joneschrisg closed 4 years ago
Right on, brother. Awesome PR.
For my own notes, which browser(s) raise on just touching window.indexedDB
and/or window.localStorage
? Brave with the latter?
Definitely Brave with window.localStorage
. Upstream Chrome can be configured similarly. I don't recall whether window.indexedDB
raised too, or we added it defensively; I believe it was defensive.
Merged! This will roll out in the next version.
One note: of
Here's the QA procedure I ran on this PR:
npm run format
npm run lint: result was clean
npm run build
npm run start
Checked that setting, getting, and removing a key works across cookies, IndexedDB, and localStorage.
what could have been better explained or better? Or, in other words, how would you improve the dev + testing loop (beyond automated tests)?
Thank you again, Chris. Don't hesitate to let me know if there's anything else I can do for you.
Thanks!!
what could have been better explained or better? Or, in other words, how would you improve the dev + testing loop (beyond automated tests)?
Automated tests are always great, of course :). Here are two smaller suggestions:
CONTRIBUTING.md
. I would recommend listing the manual QA procedure explicitly as a sequence of steps contributors should follow, with expected results.[Edit: even better, create a PR template.]
In some browsers in some contexts, just accessing
window.indexedDB
andwindow.localStorage
can throw exceptions. Ignore those exceptions so that ImmortalDB continues to work.Here's the QA procedure I ran on this PR:
npm run format
npm run lint
: result was cleannpm run build
npm run start
I committed the artifacts produced by
npm run build
--- let me know if that's done by a separate release procedure.Thanks!!