@jamesmunns would it be possible to useserde-derive here instead so that we can call the featureserde?

@yoshuawuyts I think so! It's a habit of mine to just useserde as a dependency, but it's likely not needed here, justserde-derive.

Things that need to be decided now:

1. What should the feature name be? justserde if we can depend onserde-derive? Something else?
2. Should this be behind theunstable feature? e.g. requireserde-support ANDunstable be set for now?
3. Are there any cases where a simpleflatten won't work? Maybe wrapper types with more than one field?

Once we decide and correct those, we can probably land this with just PathBuf supported, then start incrementally covering more data types. Or we can hit them all at once in this PR.

Another thing we may want to consider is adding an entry to our "features" section inlib.rs:https://docs.rs/async-std/1.0.0/async_std/#features. This seems like something people may want to be aware of.

@yoshuawuyts ah, no, I think we need aserde dependency as well. Otherwise I get:

    Checking async-std v1.0.0 (/tmp/async-std)error[E0463]: can't find crate for `serde`  --> /tmp/async-std/src/path/pathbuf.rs:22:49   |22 | #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]   |                                                 ^^^^^^^^^^^ can't find crate

Should we put serde support behind a new feature flag, perhaps namedserde1?

See also:rust-lang/api-guidelines#180

Hmm, this needs a deeper look. My first approach won't work.

use async_std;use serde_json::to_string;use serde::{self,Serialize,Deserialize};#[derive(Serialize,Deserialize)]structHmmm{ex: async_std::path::PathBuf,}fnmain(){dbg!(to_string(&Hmmm{        ex: async_std::path::PathBuf::new(),}).unwrap());}

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("can only flatten structs and maps (got a string)", line: 0, column: 0)', src/libcore/result.rs:1165:5note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

There's likely still a way, just probably a bit less elegant. Merits further investigation.

But it does work for newtypes!c70f00e switchesPathBuf to be a newtype, rather than a struct containing aninner field.

[src/main.rs:11] to_string(&Hmmm{ex: async_std::path::PathBuf::new(),}).unwrap() = "{\"ex\":\"\"}"

I don't think this needs to be a breaking change? And for fields where a newtype doesn't work, we can always manually implSerialize/Deserialize

I don't think this needs to be a breaking change?

We've never provided guarantees about internals, so indeed don't think it's a breaking change.

I think this PR looks great; but probably want to give@stjepang a chance to sign off on it too before we merge.

@stjepang just let me know if you'd preferserde orserde1 as the feature flag, then I think we can land this, and start adding serde support to other structures.

@dtolnay Do you think we should name the feature flagserde orserde1?

I am skeptical of making this a default feature. (...)

Agreed; this should only be available when theserde feature is enabled. We currently don't depend onserde anywhere else, so it's indeed a new dep.

I am skeptical of using serde_derive. (...)

Oh, yeah a manual impl def sounds better here. Thanks for suggesting!

Status?
Is anyone working on a PR to convert all wrapper types to newtypes, with serde feature?

Edit: That approach wont work in every case, as quick experiment withPath showed.
Indeed, manual impls will be needed.
The case has been brought to serde:serde-rs/serde#1913

How is the state of this?
As I understood from the conversation here, it is ready to be merged.. no?

( personally need serde support exactly for PathBuf)

It would be great to have this merged. I am also in need of this feature for PathBuf.