Skip to content

fix: make serde a non-optional dependency of pallas-primitives#779

Merged
scarmuega merged 1 commit into
mainfrom
fix/740-serde-default-features
May 21, 2026
Merged

fix: make serde a non-optional dependency of pallas-primitives#779
scarmuega merged 1 commit into
mainfrom
fix/740-serde-default-features

Conversation

@scarmuega

@scarmuega scarmuega commented May 21, 2026

Copy link
Copy Markdown
Member

Summary

pallas-primitives declared serde as an optional dependency gated behind the json feature, but the era models (lib.rs, plutus_data.rs, alonzo/model.rs, babbage/model.rs, conway/model.rs, conway/script_data.rs) use serde::{Serialize, Deserialize} and #[serde(...)] attributes unconditionally. Building the crate with default-features = false therefore failed with E0432: unresolved import \serde``.

Rather than #[cfg(feature = "json")]-gating ~90 derive/attribute sites, this makes serde a plain (non-optional) dependency:

  • pallas-codec and pallas-crypto already depend on serde unconditionally, so any consumer of pallas-primitives — even with default-features = false — already compiles serde transitively. The "optional" gating provided no real benefit; it only broke the build.
  • The json feature now gates only serde_json (the to_json API in framework.rs / alonzo/json.rs), which was already correctly #[cfg]-gated.

Why CI didn't catch this

The existing features job runs cargo check --workspace --no-default-features, but a workspace-wide build unifies features across members — other crates depend on pallas-primitives with default features, so json (and serde) was always pulled back in. This PR adds an isolated-features job running cargo check -p pallas-primitives --no-default-features, which builds the crate on its own and would have caught the regression.

Verification

  • cargo build -p pallas-primitives (default) — ok
  • cargo build -p pallas-primitives --no-default-features — ok (previously failed)
  • cargo build --workspace — ok
  • cargo test -p pallas-primitives — ok

Fixes #740

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Added CI workflow job for isolated feature validation
    • Updated dependency configuration to ensure proper feature gating

Review Change Stack

The crate gated `serde` behind the `json` feature, but used
`serde::{Serialize, Deserialize}` unconditionally across the era
models, so building with `default-features = false` failed to
compile. Since `pallas-codec` and `pallas-crypto` already depend on
serde unconditionally, gating it here gave no real benefit anyway.

The `json` feature now gates only `serde_json`. A CI job checks
pallas-primitives in isolation, since a workspace-wide
`--no-default-features` build unifies the feature back on.

Fixes #740

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 076b615a-b431-4ef3-ad1f-622c06ad2992

📥 Commits

Reviewing files that changed from the base of the PR and between 3e1738e and 21aa9e0.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • pallas-primitives/Cargo.toml

📝 Walkthrough

Walkthrough

This PR fixes and validates the issue where pallas-primitives could not be used with default-features = false due to missing serde feature gating. The serde dependency is made non-optional, the json feature is narrowed to only serde_json, and a new isolated CI job is added to catch similar issues.

Changes

Serde Dependency and No-Default-Features Support

Layer / File(s) Summary
Serde dependency restructuring
pallas-primitives/Cargo.toml
serde is changed from optional to required. The json feature now depends only on serde_json, removing the indirect serde dependency from the feature declaration.
No-default-features CI validation
.github/workflows/ci.yml
A new isolated-features job is added to run cargo check -p pallas-primitives --no-default-features with comments explaining that workspace-wide builds can unify features and accidentally enable json, so isolated crate checking is needed.

Possibly related PRs

  • txpipe/pallas#764: Modifies CI workflow build commands in .github/workflows/ci.yml; this PR adds the --no-default-features check while that PR adjusts --locked flag handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A feature gating quest we've seen,
Where serde hid between the green,
Now needed, not optional, clear,
And CI guards ensure no fear!
With checks both isolated and wise,
This crate will never need to cry. 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely describes the main change: making serde a non-optional dependency of pallas-primitives.
Linked Issues check ✅ Passed The PR fully addresses issue #740 by making serde a non-optional dependency and adding CI validation for isolated no-default-features builds.
Out of Scope Changes check ✅ Passed All changes in the PR (CI workflow addition and Cargo.toml dependency updates) are directly scoped to fixing the no-default-features compilation issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/740-serde-default-features

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@scarmuega scarmuega merged commit 705dc70 into main May 21, 2026
11 checks passed
@scarmuega scarmuega deleted the fix/740-serde-default-features branch May 21, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pallas-primitives: Unusable with default-features = false

1 participant