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

Add support for @oneOf inputs #1846

Merged
merged 34 commits into from
Jun 18, 2024
Merged

Add support for @oneOf inputs #1846

merged 34 commits into from
Jun 18, 2024

Conversation

kyri-petrou
Copy link
Collaborator

@kyri-petrou kyri-petrou commented Aug 21, 2023

VERY early draft for adding support for the @oneOf inputs, just to discuss whether we're on the right track. See current version of the RFC here

Some of the assumptions I made:

  1. We probably want to require an annotation to mark an input sealed trait as a oneOf. I think that makes sense since the spec on @oneOf is very strict, so it's very unlikely that these sealed traits will be used for any return types
  2. Since in Scala 3 we control derivation, I enforced the checks that the sealed trait is a valid @oneOf via macros. I think it's nicer UX if we give compile-time errors instead of runtime ones for invalid types. However, in Scala 2 we don't control the derivation so we have to enforce these checks during runtime. I understand if we want to unify the validation, so I'm happy to remove the macros
  3. I couldn't find anything in the spec (might have missed it though) but I assumed we don't want to mark inputs as @oneOf if they have only 1 field (kind of defeats the purpose?)

TODO:

  • Add validations for @oneOf inputs in Validator (both against the Schema & user inputs)
  • Add tests where @oneOf inputs are a mix of input objects and Scalars
  • Add documentation
  • Discuss on what we should do with the client

# Conflicts:
#	core/src/test/scala/caliban/RenderingSpec.scala
Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

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

Looks very nice!

# Conflicts:
#	core/src/main/scala-2/caliban/schema/ArgBuilderDerivation.scala
#	core/src/main/scala-3/caliban/schema/ArgBuilderDerivation.scala
#	core/src/main/scala/caliban/rendering/DocumentRenderer.scala
#	core/src/main/scala/caliban/validation/Validator.scala
#	core/src/test/scala/caliban/RenderingSpec.scala
@kyri-petrou kyri-petrou marked this pull request as ready for review September 21, 2023 01:29
# Conflicts:
#	build.sbt
#	core/src/main/scala-2/caliban/schema/SchemaDerivation.scala
#	core/src/main/scala-3/caliban/schema/SchemaDerivation.scala
#	core/src/main/scala-3/caliban/schema/macros/Macros.scala
#	core/src/main/scala/caliban/introspection/Introspector.scala
#	core/src/main/scala/caliban/introspection/adt/__InputValue.scala
#	core/src/main/scala/caliban/introspection/adt/__Type.scala
#	core/src/main/scala/caliban/schema/Schema.scala
#	core/src/main/scala/caliban/validation/Validator.scala
#	core/src/test/scala/caliban/introspection/IntrospectionSpec.scala
@kyri-petrou kyri-petrou added the breaking change The PR contains binary incompatible changes label Nov 19, 2023
# Conflicts:
#	build.sbt
#	core/src/main/scala-3/caliban/schema/DerivationUtils.scala
@kyri-petrou kyri-petrou merged commit b56673b into series/2.x Jun 18, 2024
10 checks passed
@kyri-petrou kyri-petrou deleted the one-of-input branch June 18, 2024 08:20
@tjarvstrand
Copy link
Contributor

🚀 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The PR contains binary incompatible changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants