feat: indexedDB support

[intercepted16]

Implemented indexedDB support for the persisted store. This allows data to persist in indexedDB instead of sessionStorage or localStorage, useful for larger datasets.

Key changes include:

• Localforage is now used for a consistent API
• All operations are now asynchronous due to localforage being asynchronous
• As localforage does not support sessionStorage, the default sessionStorage is used (no problems are caused)

More specific changes are listed in each commit.

PS: All operations are asynchronous, and a type warning is not provided as the methods of the persisted store do not return.

[joshnuss]

Thanks @intercepted16!

I'm not totally sure, as it it contains breaking changes. It would mean all user's of this package would need to upgrade, even if they don't use Indexed DB.

For now, I don't think it's a good time to add it. Maybe it can be considered as part of the change to Runes API. (since that will require breaking changes anyways)

P.S. Would prefer if larger changes start as discussions, because I feel bad to turn away someone's effort

[webJose]

My 2 cents: This brings an extra package. This increases bundle sizes for people that don't used indexed DB. I would vote against this even for the runes version. Perhaps it could be created as a separate package. I don't think there's enough justification to force asynchronous API's that 90+% of people won't need.

[intercepted16]

@webJose I would just like to add that localforage is actually quite lightweight; it is only 8.8kb when gzipped. However, I do agree with the forced asynchronous API's.

[webJose]

The less packages your application uses, the better in terms of maintenance. Packages become obsolete, so yoiu have to keep on updating things. Packages might also provide a surface for attacks. If you bring a package to your application that you don't need, you might end up getting extra work to clean up a CVD about it. What for? Nothing. Your app is not needing it. You are just doomed to maintain it because it comes with another package that you do use. It is good to apply the Single Responsibility Principle to packages.

[intercepted16]

@joshnuss If I modify it to use a synchronous API without localforage or an external package, would the PR be accepted?

[joshnuss]

@intercepted16 My understanding is that the IndexDB API is inherently asynchronous.

An alternative approach is having different function for each storage method.

// old-style of import would be deprecated
import { persisted } from 'svelte-persisted-store'

// instead there would be separate functions for localStorage, sessionStorage and IndexDB
import { localState, sessionState, indexDBState } from 'svelte-persisted-store'

Then the IndexDB version would have slightly different signature (async) and slightly different implementation (no need for serialization with indexdb)

[intercepted16]

@joshnuss Sorry, I made an error in my comment. I will try to implement your suggestion.

[joshnuss]

Before you implement, let me ask for feedback from a few folks

@bertmad3400 @webJose @niemyjski any thoughts on this proposed change?

[webJose]

Hello. Strictly speaking, if the feature can be tree-shaken, theoretically it would not affect bundle sizes. Using different functions as @joshnuss suggested would be the right move in the tree-shakable direction.

Still, the feature is so unique that I don't think it should be covered by this package, at least not as it is today. I don't think many people need to store a lot of data in the browser. This is, in my opinion, an edge-case feature.

A bold suggestion would be to break everything and re-think this package thinking plug-ins: Make this package provide the reactive parts, but don't provide storage of any kind. Instead, users of the package would have to install another package that would add the storage part. Therefore, users will be able to use the same "front" logic of this package with any storage they like.

Examples:

1. WebJose needs session storage. WebJose therefore installs svelte-persisted-store and svelte-persisted-store-session.
2. Joshnuss needs indexedDB storage. Joshnuss therefore installs svete-persisted-store and svelte-persisted-store-indexedDB.

Etc. A system where one package provides reactivity (the Svelte part), and another package provides the storage. Too radical? Perhaps. Still, strictly speaking such system satisfies my requirements of not increasing bundle sizes and not changing the interface.

On the other hand, indexedDB has asynchronous API. Forcing the API to synchronous is something I don't like. In the plug-in system I would then standardize all storage API to be aysnchronous. This is the downside I see.

So, practical terms: Joshnuss' suggestion seems to be the more likely one for me.

[bertmad3400]

So I have a couple of thoughts:

• This project really needs a formatter. Shouldn't be a big change, but makes diffs like these much, much more readable.
• Personally I could definitely see the use of an indexedDB option. The storage limits on localStorage means that there are some cases where that isn't viable.
• The breaking API change, by requiring everything to be async would for me (had I been the library maintainer) be a deal breaker as well. For this problem I could see two potential solutions:
  1. Implement this as an option param (so besides localStorage and session, the storage type could be indexedDB) along with some complex types, which would modify return types of the store based on the set storage type option param. This has the advantage of making this minimally invasive to the user, but would probably massively complicate the codebase.
  2. Implement this as a second store. I think this is the most viable option, though it would probably require some refactoring work of the existing code-base.
• If we are to include indexedb as an option here (and I really think we should consider that), I would love to see a draft of implementing the required interaction with indexedDB natively. I too don't like the idea of relying on localforage for this functionality, and I have a hunch that while indexedDB surely is more complicated than the localStorage API, I wouldn't believe it would be too terribly difficult.

[intercepted16]

@webJose @bertmad3400 I have implemented @joshnuss's solution of three functions; localState, indexedDBState and sessionState in a new pull request: #273