Custom storage encoding support and example

[Craig-Macomber]

An implementation of the proposed feature from #82 .

Included is an example which stores pretty printed JSON in the local storage instead of the current compressed binary format (which ironically in this example takes up much more space.

I attempted to integrated the idea in #82 (comment) and decouple the API/format from serde, but I ran into complications which can be seen in Craig-Macomber@7bda9ab . The extra type parameter introduced to constrain the encodable types leaked out to a lot of places where it caused issues, like StorageBacking.

Due to those issues, this PR contains the addition of support for custom encoding format of serde compatible types, and does not make the coupling to serde opt in.

I'd like some guidance on the preferred way for me to proceed with this work. I'm thinking I should do one of:

1. Cleanup this PR (mostly improve documentation) for merging roughly as is. This may or may not be followed up a second change to decouple this from serde.
2. Continue attempts to decouple this from serde before trying to merge this PR (In this case I'd like suggestions for what approach to take as my initial attempt didn't go very well).

Additionally I'd like clarity on how error handling should be done. For example if the storage is filled with some invalid value (possibly from a different version of the app, or collision with some other storage in a different format), I think the default behavior should probably not be to destroy that existing data, nor to panic, but instead produce an app facing error which the app can handle as desired (for example by asking the user if they want to overwrite the data or exit, or attempting to parse the data with a legacy format handler).

[ealmloff]

Thanks for working on this! The bounds are somewhat infectious, but you can get it working without serde if you just follow the errors:

main...ealmloff:dioxus-std:decouple-serde

I think some of the bounds on the current implementation are unnecessary. For example, the S: StorageBacking is only ever used in a PhantomData, so it shouldn't require the Clone bound. I wonder if this would be simpler if we removed StorageBacking and LayeredStorage and just used the new traits directly?

For error handling, it might be easier to handle some of the errors in the StorageEncoder rather than every time you read or write to the signal. If we can make it easy enough to implement StorageEncoder in userland and we allow state on the StorageEncoder, it could potentially handle fallback for deserialization failures and error reporting

[Craig-Macomber]

I finally got around to debugging an issue and getting it working on desktop: the fact that the subscriptions send the unencoded data through the channels tripped me up and I had a bug where I was sending encoded data. Still a lot of cleanup to do, but at least its working. I will continue to make progress on this.

I wonder if this would be simpler if we removed StorageBacking and LayeredStorage and just used the new traits directly?

I kept the existing stuff in place to avoid making major API breaking changes, and to limit the scope of the change. I suppose nearly all users will just be using the storage hooks with the existing storage types, so adjusting the traits wouldn't be too disruptive. I'll consider such options as I iterate on this. Reworking this might make it easier to layer the encoding better to avoid issues like the StoragePersistence.store needing the unencoded value.

[Craig-Macomber]

I think everything is working as expected now, both on web and desktop.

I still need to to do a self code review, add more documentation and add some unit tests.

[Craig-Macomber]

I found the behavior of the APIs in this file confusing given their behavior and documentation did not match.

I changed them to use LocalStorage to match the documentation, but that behaves a bit odd since they don't sync.

I'm interested on opinions on what should be done here. If there isn't a clear answer, I can split this into its own PR so it can be sorted out independently.

[ealmloff]

I think LocalStorage is correct here. That lives across page reloads unlike SessionStorage which is reset when you reload the page

[Craig-Macomber]

LocalStorage [...] That lives across page reloads unlike SessionStorage which is reset when you reload the page

Thats incorrect.

Browser SessionStorage also survives page reloads ( See https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage ):

A page session lasts as long as the tab or the browser is open, and survives over page reloads and restores.

This is part of why the current storage options are so confusing: in browser / wasm there are three reasonable lifetimes to support:

1. In memory: does not survive reloads. There is no implementation of this storage scope currently for browser, which seems like an oversight. Does not need to sync. The existing in memory storage implementation code could provide this, but is only used for desktop.
2. Session: survives reloads, but not shared between windows/tabs. As there always a single instance, does not need to sync (unless modified by something other than the Dioxus storage APIs).
3. Local (persistent and shared between tabs): might need to sync with other instances concurrently modifying it.

On Desktop we have:

1. In memory: Dioxus calls this "Session" in this case. Does not survive reloads. I find the naming of this very confusing and inconsistent with browser behavior.
2. Something that does not need sync, but survives hot reloads at least. I don't think there is anything of this type supported currently. Maybe storing to a temp file tied to PID would work for this?
3. File based storage. Dioxus calls this "Local" which seems like a reasonable choice consistent with browser behavior.

This currently do not align well with each-other.

[Craig-Macomber]

You can actually hit this if you delete the local storage entry while the web page is open. That will go on to crash on an unwrap since the current API surface doesn't handle it.

I don't think this is a regression: the old code use to unwrap before it got to this point.

In the future, we should probably have some policy of using the initial value in both empty and error cases, but have an option for the user to provide a custom handler or something. That seems like an unrelated feature and I'm considering it out of scope for now. My changes do mage it easier to implement by plumbing the errors and missing cases up to this layer however.

[Craig-Macomber]

I think this change is ready for initial review.

I ended up pivoting a bit on the design, reworking the StorageBacking trait so its easier to implement and just serves the role of composing together the encoder and persistence.

[ealmloff]

Each of the storage implementations use Value = Option<SomeValue>, but they don't really store the None variant. If I understand the code correctly, they store SomeValue but return the None value when there is an error reading the value or the value doesn't exist. I think it would be better to represent that by returning a Result<SomeValue, Self::Error> from the load method so we can expose proper error types

[Craig-Macomber]

I want storage where its possible to put the storage back into the state it was in before the application ran. This means that if the storage creates a file, it can delete it. (I am aware the current implementation of the higher level API does not yet expose a way to do this but it seems like something the storage should be able to do).

For this to work, you need to be able to differentiate between storing an empty string, and storing nothing. An option seems like the right way to do this.

If we instead use a Result, that gets really strange: it makes sense for the read code path, but gets really odd for the write code path, since you don't want to actually store something like a permissions error, but you do need to be able to store empty.

You need to be able to read and write something like an option, and ideally also have errors from either the read or write, but write returns the error, and takes in the option, while read returns the error or the option. Thus I don't think combining the error and the option works for the write case. If we want to have errors, that should be a separate thing.

On a related note, one other thing my current setup fails at that I would like to support is to provide a good way to not overwrite data which fails to decode. If the application accidentally points its storage at something it doesn't understand (for example a file in a format it does not currently support), it would be nice if there was a way to let the application decide if it doesn't want to destroy the data it could not understand, and instead error, leaving the data as is (and not just return some default and overwrite the data on next edit).

Now that I think about this, there are quite a few edge cases here where different policies might be desired. I think I'll add an extra configurable trait to handle the plumbing of the special cases (Maybe call it ErrorHandler or StorageConnector or something) to StorageBacking (alongside Encoder and Persistence). That new trait will contain the policy choices like if Errors should panic, be replaced with default values, get logged, or be passed through to the user as part of a Result. It could do the same for the underlying option in the storage, making it possible to expose a Signal<Option<T>> giving the user full control of when and if the storage entry is created or deleted, a Signal<T>, with a default value (which may or may not clear the storage in the default case), or even a full Signal<Result<Option<T>, SomeError>> if the app wants custom storage error handling UI (though such a case would probably not actually support storing the err case).

[ealmloff]

Looks much better overall. Let me know what you think about the associated error type. I can take some time to explore that approach

[rsaccon]

I would like to try out this reworked dioxus SDK storage, straight from the PR/fork. Currently I am using https://github.com/everaccountable/easy_prefs, for desktop, mobile and web. But this one integrates much nicer with dioxus, obviously. Is there an updated example anywhere already on how to use it ? Otherwise I assume it is just like using signals (but with persistence) and for fs-based persistence I need to set the directory first.

[Craig-Macomber]

Is there an updated example anywhere already on how to use it ?

This Repository contains an example of how to use the storage APIs and this PR updates it to show off the new options it adds.

[rsaccon]

Uups, sorry, now I'm on the right branch. I see, the example has also been updated.

[Craig-Macomber]

Looks much better overall. Let me know what you think about the associated error type. I can take some time to explore that approach

I'm experimenting with some ways to do error handling and storage clearing. I think I might be onto something nice. I'm going to mark this as draft again until I'm done exploring the options there.