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

Support implicit output type lookups for interface types. #600

Merged
merged 4 commits into from
Mar 26, 2021

Conversation

ocordsen
Copy link
Contributor

This is a new version of #306. I hope it maybe can be merged or perhaps it leads to another idea to solve the problem with Interface-Types that cannot be found. All context is in the original pull request so I keep it short here. :)

Copy link
Contributor

@yanns yanns left a comment

Choose a reason for hiding this comment

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

Thanks a lot!
Is it possible to add a small test case that does not compile without this change?

@ocordsen
Copy link
Contributor Author

Hey @yanns, thanks again for your time. I added a test case but maybe it only shows, what I'm doing wrong. I need the first line in the test (implicit val Parent1ObjectType = Parent1Type) to show my problem. In my current project I get an compile error, if my interface type is not an implicit val.

@yanns
Copy link
Contributor

yanns commented Mar 26, 2021

Thanks for your test.
I've tried it, with or without the implicit on Parent1ObjectType, with or without the implicit def interfaceLookup changes, and the test is always passing. I'd expect the test to fail whenever I remove implicit def interfaceLookup.

Am I missing something?

@ocordsen
Copy link
Contributor Author

Okay, that's embarrassing. I'm not sure what happend. 😅 Sorry. I updated the test to use its own types and now I get the error again, when I remove the implicit def interfaceLookup changes:

[info] - should derive a context object type with an implicit interface type and an implicit object type implementing the interface *** FAILED ***
[info]   Expected no compiler error, but got the following type error: "Can't find suitable GraphQL output type for Parent3. If you have defined it already, please consider making it implicit and ensure that it's available in the scope.", for code: deriveContextObjectType[Unit, Query, Unit](_ => new Query) (DeriveObjectTypeMacroSpec.scala:796)
[info]   org.scalatest.exceptions.TestFailedException:
[info]   at sangria.macros.derive.DeriveObjectTypeMacroSpec.$anonfun$new$386(DeriveObjectTypeMacroSpec.scala:796)
[info]   at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info]   at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info]   at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:22)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:20)
[info]   at org.scalatest.wordspec.AnyWordSpecLike$$anon$3.apply(AnyWordSpecLike.scala:1077)
[info]   at org.scalatest.TestSuite.withFixture(TestSuite.scala:196)
[info]   at org.scalatest.TestSuite.withFixture$(TestSuite.scala:195)
[info]   at org.scalatest.wordspec.AnyWordSpec.withFixture(AnyWordSpec.scala:1879)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.invokeWithFixture$1(AnyWordSpecLike.scala:1075)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.$anonfun$runTest$1(AnyWordSpecLike.scala:1087)
[info]   at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.runTest(AnyWordSpecLike.scala:1087)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.runTest$(AnyWordSpecLike.scala:1069)
[info]   at org.scalatest.wordspec.AnyWordSpec.runTest(AnyWordSpec.scala:1879)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.$anonfun$runTests$1(AnyWordSpecLike.scala:1146)
[info]   at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[info]   at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info]   at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:390)
[info]   at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:427)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[info]   at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info]   at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
[info]   at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.runTests(AnyWordSpecLike.scala:1146)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.runTests$(AnyWordSpecLike.scala:1145)
[info]   at org.scalatest.wordspec.AnyWordSpec.runTests(AnyWordSpec.scala:1879)
[info]   at org.scalatest.Suite.run(Suite.scala:1112)
[info]   at org.scalatest.Suite.run$(Suite.scala:1094)
[info]   at org.scalatest.wordspec.AnyWordSpec.org$scalatest$wordspec$AnyWordSpecLike$$super$run(AnyWordSpec.scala:1879
[info]   at org.scalatest.wordspec.AnyWordSpecLike.$anonfun$run$1(AnyWordSpecLike.scala:1191)
[info]   at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.run(AnyWordSpecLike.scala:1191)
[info]   at org.scalatest.wordspec.AnyWordSpecLike.run$(AnyWordSpecLike.scala:1189)
[info]   at org.scalatest.wordspec.AnyWordSpec.run(AnyWordSpec.scala:1879)
[info]   at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info]   at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info]   at sbt.TestRunner.runTest$1(TestFramework.scala:139)
[info]   at sbt.TestRunner.run(TestFramework.scala:154)
[info]   at sbt.TestFramework$$anon$3$$anonfun$$lessinit$greater$1.$anonfun$apply$1(TestFramework.scala:317)
[info]   at sbt.TestFramework$.sbt$TestFramework$$withContextLoader(TestFramework.scala:277)
[info]   at sbt.TestFramework$$anon$3$$anonfun$$lessinit$greater$1.apply(TestFramework.scala:317)
[info]   at sbt.TestFramework$$anon$3$$anonfun$$lessinit$greater$1.apply(TestFramework.scala:317)
[info]   at sbt.TestFunction.apply(TestFramework.scala:329)
[info]   at sbt.Tests$.$anonfun$toTask$1(Tests.scala:422)
[info]   at sbt.std.Transform$$anon$3.$anonfun$apply$2(Transform.scala:46)
[info]   at sbt.std.Transform$$anon$4.work(Transform.scala:68)
[info]   at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
[info]   at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
[info]   at sbt.Execute.work(Execute.scala:291)
[info]   at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
[info]   at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[info]   at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
[info]   at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[info]   at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
[info]   at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[info]   at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
[info]   at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
[info]   at java.base/java.lang.Thread.run(Thread.java:832)

Copy link
Contributor

@yanns yanns left a comment

Choose a reason for hiding this comment

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

No need to be embarrassed 😉
Thanks a lot for the test.

I also don't understand why scalac is not able to use implicit def outLookup[T](implicit out: OutputType[T]): GraphQLOutputTypeLookup[T].

I've tried to change it to implicit def outLookup[T, OUT <: OutputType[T]](implicit out: OUT): GraphQLOutputTypeLookup[T] but it still does not help.

👍

@yanns yanns merged commit b4e9af3 into sangria-graphql:master Mar 26, 2021
@paulpdaniels
Copy link
Contributor

Holy forgotten PRs batman! Thanks for taking up the torch on this one @ocordsen !

@yanns
Copy link
Contributor

yanns commented Apr 17, 2021

This new implicit is causing issues on our existing code base: #615

yanns added a commit that referenced this pull request Apr 17, 2021
This reverts commit b4e9af3, reversing
changes made to 1d8eab6.

Fixes #615
@yanns
Copy link
Contributor

yanns commented Apr 17, 2021

I have to revert it for now: #616
And then we can search for a better solution.

yanns added a commit that referenced this pull request Apr 17, 2021
Revert "Merge pull request #600 from ocordsen/master"
@Mxrynd
Copy link

Mxrynd commented Jun 21, 2021

Hi @ocordsen, ty for work. For me this solution works partially and I do not know if I am doing smth wrong or this should not work by design. This is model of my setup (each class/trait with companion in it's own file)

trait A {
  id: String
}

object A {
  implicit val AType = IntrerfaceType("A", fields[MyContext, A](Field(Field("id", StringType, resolve = _.value.id))))
}

case class B (id: String, b: Int) extends A

object B {
  implicit val BType = deriveObjectType[MyContext, B](Interfaces(AType))
}

case class C (id: String, с: String) extends A

object C {
  implicit val CType = deriveObjectType[MyContext, C](Interfaces(AType))
}

case class D( /*  some fields */){
  @GraphlField
  def foo(context: Context[MyContext, D]): Future[Seq[A]] = { /* some resolver call */}
}

object D{
  implicit val DType = deriveObjectType[MyContext, D]()
}

If I have AType and BType in D imported (for example, important that only 1 of BType/CType) code compiles and only fails in runtime with cannot cast exeption if smth of C present in resulting Seq. If I try to import CType than compilation fails trying to derive DType with

Can't find suitable GraphQL output type for Seq[D]. If you have defined it already, please consider making it implicit and ensure that it's available in the scope.

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.

4 participants