either icon indicating copy to clipboard operation
either copied to clipboard

serde derive feature

Open conradludgate opened this issue 2 years ago • 1 comments

Inspired by https://github.com/serde-rs/serde/issues/2584, I looked through my dep tree and saw that either includes serde with the "derive" feature.

Unfortunately, it's not so simple how to remove the feature here.

Removing derive altogether

Either uses serde for 3 things,

  1. Tagged Representation
  2. Untagged Representation
  3. Optional Untagged Representation

For 1, We can write a manual implementation of the de/serializers with no problem. For 2 and 3, the derived deserializer makes use of doc(hidden) APIs in order to cache the serde tree in case it needs to perform a second deserialisation.

Depending on serde_derive directly

dtolnay was initially against this idea because the versions of serde/derive need to be in lockstep.

There seems to be some effort to ensure it using some fancy target trickery. This is in the latest release of 1.0.186.

The trouble here is that either has 1 feature, serde. For compat with older resolvers, I think I would need to rename the serde package to serde_pkg and make the serde feature include both serde_pkg and serde_derive


Should I make a PR with the second option? The first option is possible but it requires a lot more effort to duplicate the private APIs

conradludgate avatar Aug 24 '23 08:08 conradludgate

I think the compile-time gains are way too small to justify any significant effort here, so manual implementations are out. I suppose direct serde_derive could be ok, as long as we make it clear that the remaining implicit features from optional dependencies are not considered part of the semver API. Or we could bump to MSRV 1.60 with dep: syntax, and this will also be hidden from earlier versions of the cargo resolver. (So we're not disruptive to any older MSRV crates using either, especially in public API.)

But seeing that serde-rs/serde#2584 is still open, maybe we should wait for the dust to settle a bit...

cuviper avatar Aug 24 '23 19:08 cuviper