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

serde_json: serialize None as null if there is default attribute #111

Merged
merged 3 commits into from
Sep 14, 2024

Conversation

birhburh
Copy link
Contributor

@birhburh birhburh commented Sep 2, 2024

During porting of parsing lottie files to this project from lottie-rs I comparing serialized data
and nanoserde was skipping None fields with default attribute during serialization, but serde serializes them as null

@not-fl3
Copy link
Owner

not-fl3 commented Sep 2, 2024

Thanks for PR!

I do not have a very strong opinion, but I feel like I would rather have this behavior being optional, maybe some special attribute about serializing as "null"?

@not-fl3
Copy link
Owner

not-fl3 commented Sep 2, 2024

oh, why do I always vargue in nanoserde's PRs 🤦
you are right, null is the way!

I just opened JSON standard and it says pretty clear that null is supposed to be serialized as null and nothing about removing fields.

I still feel like it would be nice to have an option to skip None's in serialized json, even if they got a default value, but maybe there should be an attribute for skipping, not for adding?

@birhburh
Copy link
Contributor Author

birhburh commented Sep 2, 2024

Checked how they skip on None in serde and of course i forgot about
#[serde(skip_serializing_if = "Option::is_none")]
Adding this to my nanoserde todo list

@not-fl3
Copy link
Owner

not-fl3 commented Sep 2, 2024

#[serde(skip_serializing_if = "Option::is_none")] looks good, but I think it would make more sense to have it another way around: skip serializing by default, and write null if the attribute is set. This way we will avoid a breaking change while support all the use cases where this null is important

@birhburh
Copy link
Contributor Author

birhburh commented Sep 7, 2024

Added serialize_none_as_null for struct and fields, and changed test accordingly

@knickish
Copy link
Collaborator

Thanks @birhburh, sorry was slow to review

@knickish knickish merged commit 0902b6d into not-fl3:master Sep 14, 2024
16 checks passed
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.

3 participants