Skip to content
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

Storage hook cannot deserialize everything serde can #20

Open
srid opened this issue Nov 8, 2023 · 6 comments
Open

Storage hook cannot deserialize everything serde can #20

srid opened this issue Nov 8, 2023 · 6 comments

Comments

@srid
Copy link

srid commented Nov 8, 2023

See #17 (comment) 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).

jamesmunns/postcard#92

@srid srid mentioned this issue Nov 8, 2023
srid added a commit to juspay/omnix that referenced this issue Nov 8, 2023
@marc2332
Copy link
Collaborator

Is this still an issue @srid ?

@srid
Copy link
Author

srid commented Apr 11, 2024

I'll check this after doing juspay/omnix#125

@ealmloff
Copy link
Member

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

@samtay
Copy link
Contributor

samtay commented Jul 14, 2024

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
Copy link
Member

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

@ealmloff
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants