wgpu-types error when using serde feature without tracing feature

[paul-hansen]

Description
The wgpu-types crate returns an error if compiled with the "serde" feature and not the trace feature.

Repro steps

cargo test --package wgpu-types --features serde

Expected vs observed behavior
I was expecting it to compile with serde serialization and deserialization of common structs.
Instead it returned many instances of this compile time error :

error: cannot find attribute `serde` in this scope                                                                                                                                                
   --> wgpu-types\src\lib.rs:123:31
    |
123 | #[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))]
    |                               ^^^^^
    |
    = note: `serde` is in scope, but it is a crate, not an attribute

Platform

• Windows 11
• wgpu master branch, commit 7495646d

Addition Info
My use case was trying to load image sampler settings from a file so I can set the texture to be repeating in Bevy. Some of wgpu's types are exposed by Bevy so I thought I would try turning on serde support to be able to load those values from a file directly. This isn't really a big deal for me, but I thought you might want to know about the inconsistency in the features.

[paul-hansen]

This is happening because Serialize and Deserialize are only derived when the trace and replay features are enabled.

wgpu/wgpu-types/src/lib.rs

Lines 1608 to 1610 in 7495646

	#[cfg_attr(feature = "trace", derive(Serialize))]
	#[cfg_attr(feature = "replay", derive(Deserialize))]
	#[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))]

Without the Serialize derive, the serde(rename_all = "kebab-case") is not valid. Which means the serde feature is fairly useless as it can't be used without one of the other features enabled.

I see a few different options so far:

1. Remove the serde feature, continue to use the trace and replay features to enable Derive and Serialize.
   Things like #[cfg_attr(feature = "serde", ... would need changed to #[cfg_attr(any(feature = "trace", feature = "replay"), ... Edit: This is impossible, serde just comes from the crate being optional.
2. Leave as is, maybe document somewhere that the serde feature shouldn't be used alone. Not ideal but it's not a big problem once you know.
3. Split the serde feature into serde_serialize and serde_deserialize so the trace and replay features can require them separately, then we can require those features for trace and replay respectively.
   #[cfg_attr(feature = "trace", derive(Serialize))] would be changed to #[cfg_attr(feature = "serde_serialize", derive(Serialize))] etc.

I'd be happy to work up a PR if desired and there's a direction you want to try.

[teoxoy]

Thanks for looking into this!
Option 3 sounds good to me.
@jimblandy @cwfitzgerald how about you guys?

[paul-hansen]

I looked at this again and wgpu-types doesn't have an explicit serde feature, it's just showing up in docs.rs because serde is optional. That seems like it might be a new feature for docs.rs or I just haven't noticed it before. So part of this was my misunderstanding thinking wgpu-types was exposing an explicit feature for serde.

Might still want to handle this in a way that doesn't error, but as it is now is more normal than I originally thought. Could still be nice to expose serialization/deserialization without enabling tracing and replay though, so option 3 still sounds like it might have some value.

[paul-hansen]

Think I'll probably close this for now unless there is more interest in it. For my own project I've just enabled trace and replay features and I haven't run into any issues, from some light poking around I didn't see anything with a lot of additional overhead that these features enable other than the de/serialization.

[teoxoy]

We should still fix this, enabling a feature but not another should not cause compilation errors.

from some light poking around I didn't see anything with a lot of additional that these features enable other than the de/serialization.

If this is the case, we should probably rename them as well.

[cwfitzgerald]

Is there a particular reason we don't just use the serde feature for everything?

[paul-hansen]

Personally I wouldn't mind if it was just one feature. I'd guess it was done to have less generated code if you don't need one?
Looks like it was done in 0e53354 if @kvark is still around maybe they can comment on if there's additional reasons etc. ?

[KirmesBude]

I would be interested in taking up this issue.
Is option 3 still the preferred approach or should I look into the suggestion from cwfitzgerald?

[paul-hansen]

Another alternative I didn't know about at the time is that you can prevent an optional dependency from generating it's own feature by using it in another feature with "dep:serde". I used this in a bevy pr here as an example: bevyengine/bevy#9516
This would remove the serde feature which removes the error case with minimal breaking changes. Essentially option 2 but using official tools. Although, if there was a desire to support my example use case more officially, then changing the features might be desirable.

Probably just need someone to make a call on what we want to try.

@KirmesBude feel free to run with this if you want, I had forgotten about it and am not working on the project I used this in anymore. Happy to help where I can though.

[KirmesBude]

Thanks for the pointers, very much appreciated.
For my usecase (I literally just want to deserialize TextureFormat) I do not really care about trace or replay.
I do not have much experience with features so I will definitely have to read up on it.

I feel like option 3 is definitely something I agree with, but it does seem to divert from the ecosystem standard by removing the "serde" feature. Or could we still have a serde feature with your "dep:serde" trick like so:
serde = ["serde_deserialze", "serde_serialize"]
serde_deserialize = ["deps:serde"]
serde_seriailize = ["deps:serde"]
trace = ["serde_serialize"]
replay = ["serde_deserialize"]