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

JsonValue type as type member #50

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

nrktkt
Copy link
Owner

@nrktkt nrktkt commented Jan 7, 2023

trait ToJsonValue[A, +Json <: JsonValue]

is now

trait ToJson[A] {
  type Json <: JsonValue
}

This means that specific JsonValue subtypes are not lost when using a ToJson[A] type annotation (instead of eg. ToJsonValue[A, JsonString]).

ToJsonValue is preserved as an aux type

type ToJsonValue[A, +J <: JsonValue] = ToJson[A] { type Json <: J }

This is especially helpful with cases like

val myClassToJson: ToSomeJson[MyClass] = ToJson.auto
myClassToJson + ("newField" -> "new value")

where object methods would not normally be available without specifying the type as ToSomeJsonObject[MyClass]

or when ToJson instances are derived from others and wish to maintain the JSON type of the instances they're derived from. For example

val _: JsonNumber = 1.toSomeJson
val _: Option[JsonNumber] = Option(1).toJson

However this has an quirk where the type of the wrapper must be annotated as a ToJsonValue like below

implicit def optionToJson[A](
    implicit toJson: ToJson[A]
): ToJsonValue[Option[A], toJson.Json] =
    ToJson((_: Option[A]).flatMap(_.toJson))

The downside is that instances can no longer be defined with lambda syntax and need to be wrapped in a ToJson.apply() constructor (although only in scala 2)


all of the above were initial thoughts. I am now thinking to scrap the whole PR. Here's why:

as stated in that last bit, you need to use ToJsonValue if you want to preserve the specific JsonValue subtype. you can also simply omit the type annotation, and type inference will take care of you. some examples that work OK

implicit def optionToJson[A](
    implicit toJson: ToJson[A]
): ToJsonValue[Option[A], toJson.Json] =
    ToJson((_: Option[A]).flatMap(_.toJson))

implicit def optionToJson[A: ToJson] = ToJson((: Option[A]).flatMap(.toJson))

however simply ToJson does not preserve the subtype

implicit def optionToJson[A: ToJson]: ToJson[Option[A]] = ToJson((_: Option[A]).flatMap(_.toJson))

this is... exactly the same as the behavior before but uses one less generic parameter. one case where that matters:

implicit def optionToJson[A: ToJson] = ToJson((_: Option[A]).flatMap(_.toJson))

would become

implicit def optionToJson[A, J <: JsonValue](implicit toJson: ToJsonValue[A, J]) = 
    ToJson((_: Option[A]).flatMap(_.toJson))

and now requires knowledge of the output type if you don't want to lose it when calling explicitly. ie. optionToJson[Int] vs optionToJson[Int, JsonNumber]

so passing this PR requires determining that the above use case is worth the trouble

@nrktkt nrktkt force-pushed the feature/type-member-rework branch from dc36de8 to 31a6524 Compare January 7, 2023 00:41
Nathan Fischer added 2 commits January 6, 2023 16:49
@nrktkt nrktkt marked this pull request as draft January 9, 2023 17:41
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.

1 participant