ghostdogpr / caliban

Functional GraphQL library for Scala
https://ghostdogpr.github.io/caliban/
Apache License 2.0
943 stars 248 forks source link

Add support for enumeratum #655

Closed ghostbuster91 closed 4 months ago

ghostbuster91 commented 3 years ago

Hi,

I would like to use enumeratum with caliban and be able to adjust my enums names using enumeratum specific technics.

So instead of that:

sealed trait RuleType

object RuleType {
  case object currency extends RuleType 

I could do:

sealed trait RuleType extends EnumEntry with Lowercase

object RuleType extends Enum[RuleType] {
  case object Currency extends RuleType //TODO caliban doesn't support enumeratum

  override def values: IndexedSeq[RuleType] = findValues
ghostdogpr commented 3 years ago

This seems to work: https://scastie.scala-lang.org/2ILMYPFnRFabZBOqifEnxA

import caliban.GraphQL._
import caliban.RootResolver
import enumeratum._
import enumeratum.EnumEntry._

sealed trait RuleType extends EnumEntry with Lowercase

object RuleType extends Enum[RuleType] {
  case object Currency extends RuleType

  def values = findValues
}

case class A(ruleType: RuleType)

val api = graphQL(RootResolver(A(RuleType.Currency)))

println(api.render)

Do you have a reproducer for something that doesn't work?

paulpdaniels commented 3 years ago

@ghostdogpr The output enum should be lowercase. Actually we have had this issue in our projects too. Basically enumeratum has a stable identifier called entryName which computes the final enum name. Honestly I'm not sure if enumeratum support should be added, but perhaps we need a type class helper that can change the enum names.

ghostbuster91 commented 3 years ago

Honestly I'm not sure if enumeratum support should be added, but perhaps we need a type class helper that can change the enum names.

Any reason why it shouldn't ? It seems like a normal integration between two libraries.

ghostdogpr commented 3 years ago

Oh I see, the issue is that it ignores with Lowercase.

A workaround if you own the type is to add the GQLName annotation:

@GQLName("currency")
case object Currency extends RuleType
paulpdaniels commented 3 years ago

Yeah certainly a work around here, I think this would just be a convenience capability.

Any reason why it shouldn't ?

Yeah we are trying to keep the number of dependencies as low as possible, enumeratum is a great library, but the more libs we add the more maintenance to keep them all up to date. Not to say it never could if the case is really compelling, but should meet with some additional scrutiny.

Playing the devils advocate here though, if we introduced an indirection layer via typeclass, then perhaps we could use the same trick we use for play and circe to add an enumeratum implicit schema?

ghostdogpr commented 3 years ago

Maybe it could live in caliban-extras, actually it is already in their TODO list 😄

valentin-panalyt commented 4 months ago

I also think that schemas should not be derived by enumeratum types. Enumeratum belongs into the domain or model part of the code. To describe the (business) domain. Caliban offers a graphql interface. Business domain and interface should be separated and to connect them, the easiest way is to simply transform scala-types into other scala-types.

What might look like duplication first is actually not really duplication. For instance, the interface types might change while the business types stay the same or the other way around. Trying to achieve that with custom derivation logic makes things very hard in the long run.

ghostdogpr commented 4 months ago

I'm closing this as it becomes even less relevant with Scala 3