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

WIP: Serde Data Format for JsValue #74

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

WIP: Serde Data Format for JsValue #74

wants to merge 16 commits into from

Conversation

jrop
Copy link

@jrop jrop commented Jun 28, 2020

This PR provides a custom serde data format for serializing/deserializing Rust data structures into/from JsValue.

Serialization Example:

#[derive(Serialize)]
struct Test {
    int: u32,
    seq: Vec<&'static str>,
}

let test = Test {
    int: 1,
    seq: vec!["a", "b"],
};
let mut map = HashMap::new();
map.insert("int".into(), JsValue::Int(1));
map.insert(
    "seq".into(),
    JsValue::Array(vec![
        JsValue::String("a".into()),
        JsValue::String("b".into()),
    ]),
);
let expected = JsValue::Object(map);
assert_eq!(to_js_value(&test).unwrap(), expected);

Deserialization Example:

#[derive(Deserialize)]
struct Test {
    int: u32,
    seq: Vec<&'static str>,
}
// deserialization example:
let mut map = HashMap::new();
map.insert("int".into(), JsValue::Int(1));
map.insert(
    "seq".into(),
    JsValue::Array(vec![
        JsValue::String("a".into()),
        JsValue::String("b".into()),
    ]),
);
let j = JsValue::Object(map);
let expected = Test {
    int: 1,
    seq: vec!["a".to_owned(), "b".to_owned()],
};
assert_eq!(expected, from_js_value(&j).unwrap());

I've been slowly working on this for a while, and -- while it is definitely not finished yet -- I thought I would open this PR and gather early feedback. I have been wanting this for quickjs-rs in my own projects, so I'm curious to see if we can 1) get this reviewed and incorporate early feedback, 2) I can work more on the outstanding items listed below, 3) merge!

TODO:

  • Cull the Error enum (initially copied from the sample serde data-format: it has more error conditions than we likely care about)
  • Add more tests
  • SerializeTupleStruct
  • JsValue::Date
  • JsValue::BigInt
  • Split the implementation into a few files (right now it is in one monolithic file)
  • Feature-gate serde

#10 #64

@jrop
Copy link
Author

jrop commented Jul 14, 2020

I took a swing at serde implementations for JsValue::Date, and I think the only way to make it work would be to create a "wrapper" type around DateTime:

  1. struct QuickJsDateTime(chrono::DateTime<Utc>)
  2. impl Deref (Target=DateTime<Utc>) for QuickJsDateTime
  3. impl Serialize for QuickJsDateTime
  4. impl Deserialize for QuickJsDateTime
  5. Change JsValue::Date(DateTime<Utc>) => JsValue::Date(QuickJsDateTime)

This would still have all of the convenience, but give us the freedom to have all of the necessary trait implementations that we will need to make this work.

@theduke I would appreciate your feedback on this conjecture!

@semirix
Copy link

semirix commented Sep 13, 2020

@theduke @jrop Any news on this? I would really benefit from this functionality being merged.

@jrop
Copy link
Author

jrop commented Sep 14, 2020

@semirix I have not spent anymore time on this for various reasons. 1) I would like @theduke to weigh in if this PR looks good so far and if my proposed changes going forward are acceptable before I spend more time on this (Some of the changes I proposed introduce potential API breakage). 2) I have been short on time as of late, due to other responsibilities.

I agree that I would love to have this integrated. It would be very useful.

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

Successfully merging this pull request may close these issues.

None yet

2 participants