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

feat: implement JsonCodecConfiguration for scala 3 and explicitEmptyCollections for JsonCodecConfiguration #1193

Merged

Conversation

ThijsBroersen
Copy link
Contributor

@ThijsBroersen ThijsBroersen commented Nov 28, 2024

PR for implementing JsonCodecConfiguration for Scala 3. Comments say it depends on softwaremill/magnolia#296 but I think it works fine like this.

This PR #932 also tries to fix this issue (I only noticed it when I was almost done). Why was that PR not finished?

closes #943

This PR also contains a new config for JsonCodecConfiguration called explicitEmptyCollections. This should make it possible to leave out empty collections or objects. I found the need for this config while working on zio-sbt when generating ci.yaml json objects which contained empty parameter maps.
The value defaults to true and that should make it fully backwards compatible.

@ThijsBroersen ThijsBroersen requested a review from a team as a code owner November 28, 2024 21:59
@ThijsBroersen ThijsBroersen marked this pull request as draft November 28, 2024 22:04
feat: implement explicitEmptyCollections for JsonCodecConfiguration
@ThijsBroersen ThijsBroersen force-pushed the feat/explicit-empty-collections-config branch from a9181d0 to 793367e Compare November 28, 2024 22:15
@ThijsBroersen ThijsBroersen marked this pull request as ready for review November 28, 2024 22:29
@@ -575,7 +590,8 @@ object DeriveJsonEncoder extends Derivation[JsonEncoder] { self =>
})
.toArray

val explicitNulls = ctx.annotations.exists(_.isInstanceOf[jsonExplicitNull])
val explicitNulls = config.explicitNulls || ctx.annotations.exists(_.isInstanceOf[jsonExplicitNull])
val explicitEmptyCollections = config.explicitEmptyCollections || ctx.annotations.exists(_.isInstanceOf[jsonExplicitEmptyCollection])
Copy link
Contributor Author

@ThijsBroersen ThijsBroersen Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some inconsistencies on the priority of JsonCodecConfiguration vs annotations, sometimes the config take priority (e.g. explicit nulls), sometimes the annotations (e.g. discriminator).
For the new config I chose to use the same pattern as explicitNulls as it also has to do with missing values. But I think it would make more sense if annotations take priority. The config first is nicer because it can overwrite conditionally.

@987Nabil 987Nabil merged commit 9339fbb into zio:series/2.x Dec 13, 2024
28 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.

[Bug] Macro configuration for Scala 3
2 participants