DioxusLabs / sdk

A library to provide abstractions to access common utilities when developing Dioxus applications.
MIT License
79 stars 12 forks source link

Storage hook cannot deserialize everything serde can #20

Open srid opened 10 months ago

srid commented 10 months ago

See https://github.com/DioxusLabs/dioxus-std/pull/17#issuecomment-1800687581 for the original issue wherein I reported that #17's use of postcard fails to deserialize certain types from disk onto Signals.

Here's a simple type that will trigger postcard's WontImplement error:

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(untagged)]
enum Foo {
    ANone,
}

Here's the code to reproduce:

        let sample =
            new_storage::<LocalStorage, BTreeMap<String, Foo>>(cx, "sample".to_string(), || {
                tracing::warn!("πŸ“¦ No sample found");
                BTreeMap::new()
            });
        tracing::info!("πŸ“¦ Sample: {:?}", sample.read());
        sample.with_mut(|s| {
            s.insert("foo".to_string(), Foo::ANone);
        });

Run the app, then quit and run it again, the console should print:

2023-11-08T15:18:54.198633Z ERROR dioxus_std::storage: Error deserializing value from storage: WontImplement
2023-11-08T15:18:54.198645Z  WARN nix_browser::app::state: πŸ“¦ No sample found
2023-11-08T15:18:54.199122Z  INFO nix_browser::app::state: πŸ“¦ Sample: {}

If we remove the #[serde(untagged)], the code works as expected. Turns out we are not the first to notice this issue with postcard:

What surprised me at first was that, even though postcard uses serde's derives, it doesn't support everything serde does for other formats. I get that there's no guarantees that a format would implement all serde features, but there are some surprises (e.g. #[serde(untagged)] will only error when deserializing, not when serializing).

https://github.com/jamesmunns/postcard/issues/92

marc2332 commented 5 months ago

Is this still an issue @srid ?

srid commented 5 months ago

I'll check this after doing https://github.com/juspay/nix-browser/issues/125

ealmloff commented 5 months ago

We still use postcard. Postcard doesn't support all of serde's features

samtay commented 2 months ago

I just got bit by this, but mine was a bit more annoying to track down. Currently no error is logged upon a deserialization failure, rather the error just gets .ok()ed into a None, so that the provided default init() value overwrites the problematic stored value. I modified dioxus-sdk locally to find an unexpected EOF deserialization error. This was caused by using the serde attribute deserialize_with on a struct field.

Can we replace the postcard serialization with just serde_json? Or perhaps offer json storage via feature flag?

jkelleyrtp commented 2 months ago

We should switch to cbor or bincode which are binary encoding formats that support more serde features

ealmloff commented 2 months ago

cbor would be better for binary sizes because it is already pulled in for fullstack