Support implicit output lookups for interface types

[paulpdaniels]

For context from the gitter channel.

Hey all, I seem to be having a problem with the deriveContextObjectType when one of the derived type's methods returns an interface type. I receive a Can't find suitable GraphQL output type for services.tag.GenericTag. If you have defined it already, please consider making it implicit and ensure that it's available in the scope. Error during compilation instead. Is this a known limitation? It seems to work if I switch the return type to a concrete type...

e.g.

// In Mutation.scala
trait Mutation {
   @GraphQLField
   addTag: Future[GenericTag] = {} // GenericTag has a GraphQL InterfaceType declaration

}

// in SchemaDefinition.scala

implicit val TagType = InterfaceType("Tag", "",  () => fields[UserContext, GenericTag]()
implicit val ConcreteTagType = deriveObjectType[UserContext, GenericTag](Interfaces(PossibleInterface[UserContext, ListTag](TagType)))

val MutationType = deriveContextObjectType[UserContext, Mutation, Unit](
    _.mutation,
    IncludeMethods("addTag"),
    Interfaces(PossibleInterface[UserContext, Unit](TagType))
  )

The problem seemed to be that scala has a hard time determining the implicit GraphQLOutputTypeLookup for an InterfaceType.

[OlegIlyenko]

Thanks! It still puzzles me a bit since InterfaceType is a subtype of OutputType. But if it helps scala compiler to find the appropriate interface type, then let's try it 👍

[OlegIlyenko]

A quick question. According to the definitions in the example:

implicit val TagType: InterfaceType[UserContext, GenericTag] = ...
implicit val ConcreteTagType: ObjectType[UserContext, GenericTag] = ...

Which can be reduced down to:

implicit val TagType: OutputType[UserContext, GenericTag] = ...
implicit val ConcreteTagType: OutputType[UserContext, GenericTag] = ...

What is the difference between TagType and ConcreteTagType? They both defined in terms of GenericTag, isn't it? Why should scala prefer over another? (these look like ambiguous implicits to me)

[OlegIlyenko]

After another thought, I temporarily reverted the change. Today I would like to make a release with several important spec changes and one critical performance improvement. I would prefer not to delay today's release and also not to rush this change in order to better understand its implications.

I always cautious about new generic implicits since they often have a big impact on scala compiler (more often than not, in very unintuitive ways). I would suggest proceeding with following plan: let's create another PR with the proposed change and in addition to it let's add a test case that would demonstrate and test this implicit (this test probably should not compile without proposed change). I think it would be very helpful. I think DeriveObjectTypeMacroSpec would be a suitable place for such test. There are also a lot of other tests in there that can be used as a template.

What do you think about it?

[paulpdaniels]

Lol I was planning to do a test case, but I wanted to make sure that was the correct approach to take. Anyway, I'll re-open a new PR with a test case for this, once I figure out how to get the tests to run on windows correctly.

[OlegIlyenko]

If you are using windows, then I think there a 2 test files that rely of the carriage returns because they assert on AST node positions (you just need to make sure that these two test files have linux-style carriage returns LF instead of CRLF)

[fnmeissner]

Hi! I also encountered this problem. The code from @paulpdaniels fixed the problem for me, thanks for the pr:

implicit def interfaceLookup[T](implicit int: InterfaceType[_, T]) = new GraphQLOutputTypeLookup[T] {
    override def graphqlType = int
}

As a workaround, I just pasted the lines next to my implicit types.

[kusmierz]

I guess this PR should be reopened...

[ocordsen]

Hi, I still have the problem, that I need the above lines of code if I introduce an InterfaceType. Is this "by design" or could it be fixed in Sangria? @yanns would you mind to take a look? (I hope it's okay to mention, I'm not sure if there is a notification otherwise.)

Edit: I'm using version 2.1.0.

[yanns]

Feel free to open a new PR with those changes so that we can check them. ❤️

[ocordsen]

Thanks for your friendly response. I created a pull request as mentioned above.

[sh0hei]

Thank you for PR @ocordsen !
It will be reviewed by sangria-team. Please wait 🙏