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

Implement inheritDiscriminator annotation #1113

Closed
wants to merge 4 commits into from

Conversation

pablf
Copy link
Member

@pablf pablf commented May 12, 2024

This is a different approach than #1112. The PR implements a inheritDiscriminator annotation to be used on case classes. If the parent class doesn't have a jsonDiscriminator annotation it will throw a compilation error when deriving a codec. When there is a jsonDiscriminator it will include this in the encoding with the name of the case class or the jsonHint.

I don't know if this is the exact use case of @alphaho, but I guess it might be encoding and decoding with different encoders for convenience when interacting between different parts of the code base. In these cases it might be easier to use something like this annotation. And this isn't a breaking change. What do you think @987Nabil?
fixes #1056
/claim #1056

The final code for the example of @alphaho would be

import zio.json._

@jsonDiscriminator("type")
sealed trait Animal
object Animal {
  @inheritDiscriminator @jsonHint("dog")
  case class Dog(name: String) extends Animal
  object Dog {
    implicit val dogCodec: JsonCodec[Dog] = DeriveJsonCodec.gen
  }
  
  @inheritDiscriminator @jsonHint("cat")
  case class Cat(name: String, weight: Double) extends Animal

  implicit val animalCodec: JsonCodec[Animal] = DeriveJsonCodec.gen
}

@987Nabil
Copy link
Contributor

My personal opinion is, that this is a very special solution for a probably fringe use case that is also possible to have with more generic solution that already exists.
I would not merge this.
Maybe @fsvehla or @jdegoes can give their opinion too.

@jdegoes
Copy link
Member

jdegoes commented May 12, 2024

@pablf In your example, do you really need:

    implicit val dogCodec: JsonCodec[Dog] = DeriveJsonCodec.gen

or is this a copy/paste error?

@987Nabil
Copy link
Contributor

@jdegoes the purpose was to have a Dog decoder that fails without the discriminator. How would this work, if you don't create one explicitly?

@pablf
Copy link
Member Author

pablf commented May 14, 2024

@jdegoes I think so. I don't understand completely the exact needs behind the feature request. Possibly the approach with map or transform should be enough for the use case of @alphaho.

@alphaho
Copy link

alphaho commented Dec 14, 2024

Very sorry for the late reply. While the inheritDiscriminator feels handy to me, I totally agree that this may be a special solution for such a use case. The approach from #1112 may be good enough for this case. When I came across the problem, I was so into the mindset that I want to generate the JsonCodec instance and I forgot that I can actually get one by mapOrFail on the parent's.
Thank you for both of your efforts by working on this issue! ❤️❤️ @987Nabil, @pablf

@987Nabil 987Nabil closed this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] support jsonDiscriminator in case class
4 participants