Skip to content

Add impl From<()> for Value#585

Merged
dtolnay merged 1 commit intoserde-rs:masterfrom
Nilix007:add_from_unit_for_value
Nov 24, 2019
Merged

Add impl From<()> for Value#585
dtolnay merged 1 commit intoserde-rs:masterfrom
Nilix007:add_from_unit_for_value

Conversation

@Nilix007
Copy link
Copy Markdown
Contributor

@Nilix007 Nilix007 commented Nov 19, 2019

Hey there,

this tiny PR adds the conversion from () to Value::Null.

I feel this is justified as serde_json already perceives () as the JSON value null:

println!("{:?}", serde_json::from_str::<()>("null"));
// prints `Ok(())`
    
println!("{:?}", serde_json::to_string(&()));
// prints `Ok("null")`

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=0eac935756f3537acdf45486dcabb4db

Cheers,
Felix

PS: Please have a look at the marvelous signature of <Value as From<()>>::from 😆

dtolnay
dtolnay reviewed Nov 19, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share an example where having this impl would be helpful?

@Nilix007
Copy link
Copy Markdown
Contributor Author

Nilix007 commented Nov 19, 2019 •
edited
Loading

Mh. So I was working on a library which uses diesel's Jsonb internally for some kind of dynamic data field. As Jsonb maps directly to Value, I'm exporting functions like fn do_something<T: Into<Value>>(additional_data: T) {} so that I can avoid pulling in the complete serde dependency tree and skip the serialization step completely (yay, one error case less on my side due to type safety).

Anyway, long story short, I think that the conversion from this PR would be quite convenient for the end user of my lib. And as it kinda feels "natural" to me, I sent this PR.

@dtolnay
Copy link
Copy Markdown
Member

dtolnay commented Nov 19, 2019

What do you mean by "pulling in the complete serde dependency tree"? If you depend on serde_json to use Value, that depends on serde anyway.

Do you prefer Into<Value> over:

fn do_something(additional_data: impl Serialize)

which would be more flexible?

@Nilix007
Copy link
Copy Markdown
Contributor Author

Nilix007 commented Nov 20, 2019

What do you mean by "pulling in the complete serde dependency tree"? If you depend on serde_json to use Value, that depends on serde anyway.

Err, you're right, of course. Please ignore this part 😬

Do you prefer Into over:

fn do_something(additional_data: impl Serialize)

which would be more flexible?

Yes, because serde's serialization (and serde_json::to_value respectively) can fail while From/Into cannot.

dtolnay
dtolnay approved these changes Nov 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit bf8cc66 into serde-rs:master Nov 24, 2019
@dtolnay
Copy link
Copy Markdown
Member

dtolnay commented Nov 24, 2019

Published in 1.0.42.

takumi-earth pushed a commit to earthlings-dev/json that referenced this pull request Jan 27, 2026
@dtolnay
Merge pull request serde-rs#585 from Nilix007/add_from_unit_for_value
a1c4345 
Add `impl From<()> for Value`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Nilix007 @dtolnay