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

Preserved value de-serialization error #59

Open
FredrikNoren opened this issue Dec 12, 2023 · 8 comments
Open

Preserved value de-serialization error #59

FredrikNoren opened this issue Dec 12, 2023 · 8 comments

Comments

@FredrikNoren
Copy link

Hi,

I'm trying to use the js value preservation, but I'm getting an error. For this code:

#[serde(with = "serde_wasm_bindgen::preserve")]
    pub created: JsValue,

I'm getting the following error:

Error: Error: invalid type: JsValue(Object({"_seconds":1702379961,"_nanoseconds":123000000})), expected tuple struct PreservedValueDeWrapper

Any ideas?

@RReverser
Copy link
Owner

RReverser commented Dec 12, 2023

None, sorry. It works in tests as expected. It would be helpful if you could provide an isolated minimal repro - e.g. construct a test object using JsValue methods or using JSON::parse and then parse back into a struct with a preserved value and assert_eq the result.

@RReverser
Copy link
Owner

Actually, I do have one guess - which is the most common source of all issues in Serde - are you using serde(flatten) by any chance?

@FredrikNoren
Copy link
Author

Hm it wasn't flattened. But I can't reproduce it right now, I'm not sure exactly what happened. I'll close for now and re-open if I can find some more info. Thanks meanwhile!

@FredrikNoren
Copy link
Author

Ok I ran into this error again and found the problem; I was using tsify and they're on an older version of serde-wasm-bindgen. Added a PR to fix: madonoharu/tsify#48

@pierd
Copy link

pierd commented Feb 2, 2024

Managed to reproduce it.

This seem to happen when preserved is used in a nested type, for example this works fine:

#[wasm_bindgen_test]
fn simple_preserved_value() {
    #[derive(serde::Deserialize, serde::Serialize, PartialEq, Clone, Debug)]
    struct A {
        #[serde(with = "serde_wasm_bindgen::preserve")]
        x: JsValue,
    }

    let case = A { x: "test".into() };
    let passed_through: A = from_value(to_value(&case).unwrap()).unwrap();
    assert_eq!(case, passed_through);
}

But using struct A inside an enum for example like this:

#[wasm_bindgen_test]
fn nested_preserved_value() {
    #[derive(serde::Deserialize, serde::Serialize, PartialEq, Clone, Debug)]
    #[serde(tag = "type")]
    enum SomeEnum {
        A { a: A },
    }
    
    #[derive(serde::Deserialize, serde::Serialize, PartialEq, Clone, Debug)]
    struct A {
        #[serde(with = "serde_wasm_bindgen::preserve")]
        x: JsValue,
    }

    let case = SomeEnum::A { a: A { x: "test".into() } };
    let passed_through: SomeEnum = from_value(to_value(&case).unwrap()).unwrap();
    assert_eq!(case, passed_through);
}

causes an error like the one mentioned in the first comment:

---- node::common::nested_preserved_value output ----
    error output:
        panicked at tests/common/mod.rs:731:73:
        called `Result::unwrap()` on an `Err` value: Error(JsValue(Error: invalid type: string "test", expected tuple struct PreservedValueDeWrapper
        Error: invalid type: string "test", expected tuple struct PreservedValueDeWrapper
...

@FredrikNoren FredrikNoren reopened this Feb 2, 2024
@pierd
Copy link

pierd commented Feb 2, 2024

This seems to be the case only for internally tagged enum representation. Works as expected for nested structs or externally/adjacently tagged enums.

@RReverser
Copy link
Owner

Ah yeah this is an unfortunate side effect of another ancient Serde issue: serde-rs/serde#1183

TL;DR attributes like serde(tag) and serde(flatten) collect inputs into Serde's internal AST first, and only then try to parse that with the target deserializer. Unfortunately, this transformation is lossy and often breaks this kind of advanced features (see the number of linked issues in the referenced one).

@RReverser
Copy link
Owner

Maybe we can relax our check somehow to support this particular case (even though it will still break on more complex inputs), need to think.

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

3 participants