Another way to generate schema for @oneOf type

[ETCHKILI]

Hi, I've been using this library for a while and it's pretty good, but today I'd like to discuss an improvement about schema generation of @oneOf.
According to https://github.com/graphql/graphql-spec/pull/825/files, the @oneOf is merged to main line of graphql-spec.
So I think now is a good time to talk about schema generation of @oneOf.

Currently, according to the doc, the type def below

case class Name(firstName: String, lastName: String)

@GQLOneOfInput
sealed trait AuthorInput
object AuthorInput {
  case class ById(id: String)
  case class ByName(name: Name)
}

will generate schema below

input NameInput {
    firstName: String!
    lastName: String!
}

input AuthorInput @oneOf {
    id: String
    name: NameInput
}

,
which brings limitation that, every kind of AuthorInput should contains exactly one field that is different from others.
Of course we can just wrap fields into new case classes, but that's a bit not ergonomically friendly.

But if we provide an alternative, which generate schema below

input ById {
    id: String!
}

input ByName {
    firstName: String!
    lastName: String!
}

input AuthorInput @oneOf {
    ById: ById
    ByName: ByName
}

for type def below

@GQLOneOfInput
sealed trait AuthorInput
object AuthorInput {
  case class ById(id: String)
  case class ByName(firstName: String, lastName: String)
}

We can enjoy a more ergonomic model derivation.

This is just a preliminary idea, but if it's feasible, I'd love to help implement this feature.
What do you think?

[ghostdogpr]

The issue with that approach and the reason it requires a wrapper case class is that the oneof field can be just a scalar such as id: String which can't be expressed with your proposal.

Now we could have another annotation and support both ways but I'm a bit wary of multiplying annotations. @kyri-petrou thoughts?

[kyri-petrou]

As @ghostdogpr mentioned the part that makes it tricky is including scalars.

One potential solution would be to add a boolean param to the @GQLOneOfInput annotation as:

// For lack of better arg name
case class GQLOneOfInput(deriveAsValueTypes: Boolean = true) extends StaticAnnotation

And then one could potential do this:

@GQLOneOfInput(deriveAsValueClasses = false)
sealed trait AuthorInput
object AuthorInput {
  case class ById(id: String) extends AuthorInput
  case class ByName(firstName: String, lastName: String) extends AuthorInput
}

Potentially, we could also allow for scalars to be expressed as well by annotating them with @GQLValueType. e.g., in the example above:

@GQLValueType case class ById(id: String) extends AuthorInput

This might end up being quite confusing for users though, so if we were to do this we'd need to document it very well both in the docs and in the scaladoc of the GQLOneOfInput annotation

[ETCHKILI]

Can we make the leave case class decide its schema?

for example, add deriving for Scalar type

sealed trait AuthorInput derives Schema.SemiAuto
object AuthorInput {
  case class ById(id: String) extends AuthorInput
  // for example, be a string scalar
  given Schema[ById] = Schema.scalarSchema(???)
  case class ByName(firstName: String, lastName: String) extends AuthorInput
}

Then the derives of AuthorInput should respect the implicit search of schema of leaves.
So the generated schema might be something like

input ByName {
    firstName: String!
    lastName: String!
}

input AuthorInput @oneOf {
    ById: String!
    ByName: ByName
}

The derivation will be like circe.
I'm not sure if it can work. What do you think of this approach? @ghostdogpr @kyri-petrou

[ghostdogpr]

Potentially, we could also allow for scalars to be expressed as well by annotating them with @GQLValueType. e.g., in the example above:

@GQLValueType case class ById(id: String) extends AuthorInput

This might end up being quite confusing for users though, so if we were to do this we'd need to document it very well both in the docs and in the scaladoc of the GQLOneOfInput annotation

I think it's not bad actually? @ETCHKILI its similar to your suggestion but with less boilerplate as you don't need to define the schema manually and its reusing some existing behavior.

The main issue is that it'd be a breaking change if we keep the original annotation but maybe it's okay with clear release notes and the migration path is easy.

[ETCHKILI]

Yeah I got it. I think I'll try to finish it.