-
Notifications
You must be signed in to change notification settings - Fork 320
Re-export serde_json::Value from core #3296
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
Conversation
Resolves Azure#1687. As long as we use `Value` from core, should `serde_json::Value` ever make a breaking change to `Value` we can effectively vendor just that while continuing to pick up changes to the de/serializers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes serde_json an optional dependency controlled by the json and xml features in typespec_client_core, and re-exports serde_json::Value as Value from both typespec_client_core and azure_core for easier consumption. The changes ensure that JSON-related functionality is only compiled when needed, improving modularity and compilation times for consumers that don't require JSON support.
Key changes:
- Made
serde_jsondependency optional, enabled by eitherjsonorxmlfeatures - Re-exported
serde_json::Valueas a top-level type in both crates - Added feature gates throughout the codebase to conditionally compile JSON-related code
- Updated all references to use the re-exported
Valuetype instead ofserde_json::Valuedirectly
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/core/typespec_client_core/Cargo.toml | Made serde_json optional and tied to json/xml features |
| sdk/core/typespec_client_core/src/lib.rs | Re-exported Value conditionally and added feature gate for Sealed trait impl |
| sdk/core/typespec_client_core/src/http/response.rs | Added conditional compilation for Response struct based on json feature |
| sdk/core/typespec_client_core/src/http/request/mod.rs | Added feature gates for JSON-related imports, methods, and types |
| sdk/core/typespec_client_core/src/http/method.rs | Simplified Display trait implementation to be unconditional |
| sdk/core/typespec_client_core/src/http/format.rs | Added feature gates for JsonFormat and its implementations |
| sdk/core/typespec_client_core/src/base64.rs | Added feature gates for JSON serialization/deserialization tests |
| sdk/core/typespec_client_core/CHANGELOG.md | Documented the new re-exported Value type |
| sdk/core/typespec/Cargo.toml | Made serde dependency required for xml feature |
| sdk/core/azure_core/src/lib.rs | Re-exported Value from typespec_client_core |
| sdk/core/azure_core/src/error/error_response.rs | Updated to use crate::Value instead of serde_json::Value |
| sdk/core/azure_core/benches/deserialization.rs | Updated comments and imports to use azure_core::Value |
| sdk/core/azure_core/CHANGELOG.md | Documented the new re-exported Value type |
Resolves #1687. As long as we use
Valuefrom core, shouldserde_json::Valueever make a breaking change toValuewe can effectively vendor just that while continuing to pick up changes to the de/serializers.