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

Unexpected uint error with newtype wrapper #45

Open
wg opened this issue Apr 6, 2024 · 2 comments
Open

Unexpected uint error with newtype wrapper #45

wg opened this issue Apr 6, 2024 · 2 comments

Comments

@wg
Copy link

wg commented Apr 6, 2024

Hello!

I have a newtype wrapper around a u64 like so:

#[derive(Debug, Hydrate, Reconcile)]
pub struct Item {
    pub name: String,
    pub date: Option<Date>,
}

#[derive(Debug, Hydrate, Reconcile)]
pub struct Date(u64);

When I attempt to hydrate it from a doc with a value present for that key, I get the error unexpected uint.

I think this is because Option::hydrate calls Date::hydrate_uint but the derive impl for a newtype struct only generates Hydrate::hydrate. Is there a better way to accomplish what I'm trying to do here?

And as a larger meta question, why isn't the Hydrate impl for Option simply something like this that handles both missing and explicitly null values?

Ok(match doc.get(obj, &prop)? {
    Some((Scalar(v), _)) if *v == Null => None,
    Some(_) => Some(T::hydrate(doc, obj, prop)?),
    None    => None,
})
@alexjg
Copy link
Collaborator

alexjg commented Apr 6, 2024

The reason Option<T>::hydrate returns an error if the value is not present at all is that sometimes you need to know that there wasn't a value in the document. Schema migrations are actually a good example of this, you might introduce a field which is optional but which you wish to have a default value for when the document was created before the field was introduced; in this case you would need to distinguish between the value being Null vs not present at all.

We have two tools for managing this. First there's MaybeMissing which is a wrapper type which allows you to detect the missing value. Second there is the missing= attribute which allows you to specify a function which will be used to populate the missing value (in your case you can use Default::default() to get the None).

@wg
Copy link
Author

wg commented Apr 7, 2024

Having a mechanism to detect and handle missing values makes sense, thanks! However this default behavior for Option is very surprising, Rust (serde)1, Swift2, and Go3 all support deserializing both { "name": "test" } and { "name": "test", "date": null } into a struct with optional date field, where the date will be None/nil. TypeScript with zod's nullish behaves similarly.

I think optionality is orthogonal to defaults, when I want a default value in Rust I use the Default trait, not Option. Wouldn't it be better to make the missing= attribute an opt-in feature similar to serde's default attribute? As it stands right now I'd have to add #[autosurgeon(missing="Default::default")] to every single optional field in my schema which adds a lot of noise.

Footnotes

  1. https://gist.github.com/wg/a5322a581cc7f770763e649e1040aafc

  2. https://gist.github.com/wg/266811a8c9ca2964a3d83ea8d4a8a657

  3. https://gist.github.com/wg/db5b22a5aaa494be5f929fb4ce70d002

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

2 participants