-
Notifications
You must be signed in to change notification settings - Fork 147
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
utility crate: local and session storage #3
Comments
I will work on it |
Just FYI, the local storage APIs are heavily discouraged, because they are synchronous, which means they fully block page loading. This causes significant performance issues. Instead, the recommendation is to use IndexedDB, or the new experimental kv-storage API (not stabilized, and Chrome only for now). |
I think it makes sense to have what we've dubbed "mid-level" APIs for local storage, even if IndexedDB is preferred. At minimum, it will make writing spec-compliant TodoMVC for various frameworks easier 😆 I don't think we should build libraries for anything that is still experimental / not cross-browser supported and part of the open Web yet (like the kv-storage API). But yeah, would also love to get the ball rolling on Indexed DB too! |
I'm not sure if we should be encouraging bad APIs. Especially APIs with really bad performance (Rust is supposed to be fast!), especially when there's a better alternative. Instead, I think we should just make the IndexedDB API really good and convenient, so nobody feels the need for local storage. Using Also, what would a "mid-level" API for local storage even look like? Here's how it looks like using raw let store = window().unwrap().local_storage().unwrap().unwrap();
store.set_item("foo", "bar").unwrap(); Aside from all the
Agreed, I just brought it up because it will (eventually) be replacing local storage, and it also showcases why local storage is bad. |
Indeed! This is a great vision!
Hm, yeah I guess there isn't really anything for a mid-level API to improve upon. A higher-level API could allow transparent serialization and deserialization to and from storage. Overall, I agree that effort is better probably spent on Indexed DB (although if @c410-f3r is only interested in tackling local/session storage, I think it is worth accepting PRs for it and not rejecting them). |
The nanoidb crate might be a good reference for an IndexedDB design. In particular one gotcha to watch out for when working with IndexedDB is that transactions / batched changes are automatically persisted to the database at the end of the current event loop tick. If not careful this means that you could be trying to persist data to the database after the current transaction has completed, creating some awkward situations. The way we designed around that was by requiring a |
It's possible to emulate linear types using closures: db.transaction(|tx| {
tx.push(...);
// At the end of the closure the tx is committed
}) Since (If needed, we can provide an escape hatch that allows for the usage of a transaction outside of a closure, but that should have a lot of warnings in the docs). |
@Pauan ohhhh, TIL! -- this is very cool; thanks for sharing! |
@yoshuawuyts Rust used to have a However, this API caused undefined behavior because it was not guaranteed that the thread guard would be dropped. So it was removed and instead a new API was created to access references across threads. This new API is safe because it uses a closure to statically restrict the lifetime of the variables. So it's another example of using closures to emulate linear types. I've found that many issues involving lifetimes in Rust are solved with closures. |
Thanks for this informative conversation that will certainly improve the API design. |
Note that the closure-based api is necessary because the scoped thread cleanup must happen for memory safety. When memory safety is not being enforced, then normal |
Fixed in #125 |
We should have a utility crate for working with
window.sessionStorage
andwindow.localStorage
!The text was updated successfully, but these errors were encountered: