Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Depend directly on serde_derive #130

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hanna-kruppe
Copy link

@hanna-kruppe hanna-kruppe commented Mar 1, 2024

Since serde v1.0.186, this is possible without risking errors from Cargo picking incompatible versions of serde and serde_derive. The benefit of not enabling serde's "derive" feature is that (non-incremental) builds can become a bit faster, as serde itself (and some things depending on it) can be compiled in parallel to proc-macro2, syn, and serde_derive. See serde-rs/serde#2584 for more discussion and background. In a project of mine where all transitive dependencies except postcard have already made the switch, this patch makes a clean build 3-4 seconds faster (out of a minute in total) on my machine.

The downside of this change, besides the additional use lines, is that it's easy to accidentally write code that breaks when used in a project where something else enables serde's "derive" feature. In scopes where the derive macros are imported from serde_derive, importing the traits as serde::{Derive, Serialize} seems to work until it doesn't (see serde-rs/serde#1441). I think avoided that problem in this PR, but there's no good way to test it other than occasionally editing Cargo.toml to re-enable serde's "derive" feature. I'm honestly not sure if I'd want to deal with that as a library maintainer, no hard feelings if you don't want to either.

The second commit goes one step further and makes postcard's serde_derive dependency completely optional except as a dev-dependency, since it only seems to be needed for tests and for postcard-derive-related code in schema.rs. This is less impactful since virtually every crate depending on postcard also needs serde_derive anyway, but it shouldn't hurt either and doesn't make things any more complicated than the first commit.

This is a breaking change: projects using postcard are likely to depend on serde re-exporting the derive macros, and for some of them, postcard will be the only thing enabling serde's "derive" feature. But since a 2.0 release is likely coming anyway, I wanted to pitch this change.

Outside of tests, the only use of serde_derive is in schema.rs, which
is only exposed when postcard's experimental-derive feature is enabled.
Copy link

netlify bot commented Mar 1, 2024

Deploy Preview for cute-starship-2d9c9b canceled.

Name Link
🔨 Latest commit d8174d7
🔍 Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/65e2470518df760008dd0d06

@jamesmunns jamesmunns added the postcard-2.0 Tracking issues for an eventual 2.0 version of the postcard wire format (not currently planned) label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
postcard-2.0 Tracking issues for an eventual 2.0 version of the postcard wire format (not currently planned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants