@jamesmunns would it be possible to use serde-derive here instead so that we can call the feature serde?

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

Things that need to be decided now:

1. What should the feature name be? just serde if we can depend on serde-derive? Something else?
2. Should this be behind the unstable feature? e.g. require serde-support AND unstable be set for now?
3. Are there any cases where a simple flatten 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 in lib.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 a serde 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 named serde1?

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)]
struct Hmmm {
 ex: async_std::path::PathBuf,
}
fn main() {
 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:5
note: 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 switches PathBuf to be a newtype, rather than a struct containing an inner 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 impl Serialize/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 prefer serde or serde1 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 flag serde or serde1?

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

Agreed; this should only be available when the serde feature is enabled. We currently don't depend on serde 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 with Path 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.