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

"Field list is empty" when only using @GraphQLField via deriveContextObjectType #172

Closed
tekacs opened this issue Oct 23, 2016 · 18 comments

Comments

@tekacs
Copy link

commented Oct 23, 2016

Example:

trait Query {
  val db: Our.Db
  import db._

  @GraphQLField
  def allUsers: Seq[User] = db.run(quote(query[User]))
}

object Query {
  case class Ctx(query: Query)
  implicit val UserOutType = deriveObjectType[Ctx, User]()
  implicit val queryType = deriveContextObjectType[Ctx, Query, Unit](_.query)
  val schema = Schema(queryType)
}

This works just fine if I use:

implicit val queryType =
  deriveContextObjectType[Ctx, Query, Unit](_.query, IncludeMethods("allUsers"))

I've tried reading over the macro a time or two but the cause doesn't jump out at me.

Any ideas?

@tekacs

This comment has been minimized.

Copy link
Author

commented Oct 23, 2016

As a weird aside - if I use IncludeMethods, then remove it and let the Play framework recompile it incrementally, Sangria doesn't seem to notice that it's now gone and keeps working... until I recompile from scratch. 👀

@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Oct 23, 2016

Yesterday I myself faced precisely the same issue :) I tried to investigate it and debugged the macro... Turns out that in some cases scala API does not return the list of annotations at all - it's just empty, even if I have annotations on the field or method. I tried different things and nothing seem to work. Then I moved the classes (without any modifications) to a different package and suddenly scala API started to return the correct list of annotations 🙃

I still a bit confused with this behavior. I can try to report it since it looks like a bug in scala compiler (at the end of the day, scala macros are still experimental). I can also try to use java annotations instead of scala's StaticAnnotation which sangria uses now. I will continue investigating this.

@tekacs

This comment has been minimized.

Copy link
Author

commented Oct 23, 2016

Thanks for looking into this - I've experienced similar bizarro issues with Scala macros before (I'll see if I can dig up any resolution or comments from others that I found).

I wonder if any other macro-heavy library authors would know about this bug and perhaps be aware of an existing report or a mitigation - shapeless or Quill, perhaps?

🙃 indeed!

As many other things as there might be that need fixing with scala.meta, at least it should be guaranteed to return an exact parse of the source code at all times, for things like annotations! 😛

@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Oct 23, 2016

Thanks for the hint! I think it would be good idea to ask for advice in other communities that also heavily use macros.

As many other things as there might be that need fixing with scala.meta, at least it should be guaranteed to return an exact parse of the source code at all times, for things like annotations!

Indeed there are quite a few issues with macros where the results sometimes differ based on a compilation order :( I hope scala.meta will address these issues

@tekacs

This comment has been minimized.

Copy link
Author

commented Oct 30, 2016

This is my mitigation, for anyone who comes across this issue before a solution is found - this simply includes all methods on a class for derivation automatically, with explicit excludes, rather than only those marked by @GraphQLField.

https://gist.github.com/tekacs/78c2ee6fff12773dcc69d004a9d6fcf2

@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Nov 5, 2016

In the latest version, i think I found the root cause of this issue. As I migrated to scala 2.12, this issue became more reproducible. Turns out that annotations are also provided in a case class constructor (and in scala 2.12 it's actually the only place where one can discover them). I you suggest you to update to thi version:

https://github.com/sangria-graphql/sangria/releases/tag/v1.0.0-RC3

Meanwhile I will close this issue, since I would consider it to be resolve. but please feel free to reopen it if you face the same issue with the new version.

@OlegIlyenko OlegIlyenko closed this Nov 5, 2016

@tekacs

This comment has been minimized.

Copy link
Author

commented Nov 5, 2016

In my own workaround code I wasn't running into this issue and was wondering why - I was accessing the case class constructor and hadn't noticed that Sangria wasn't. 😄

I'll update and see how it goes, but given that annotations were reliable this way for me, I'm hopeful! Thanks!

@pahomovda

This comment has been minimized.

Copy link

commented Nov 26, 2016

Issue is still presents in 1.0.0-RC4

I have folowing setup

case class Ctx(...) extends Query with Mutation {
  ...
}

trait Query extends UserQuery { this:  Ctx =>
}

trait UserQuery { this:  Ctx with Query  =>
  
  @GraphQLField
  def users(...) = ...
}

When i use implicit val queryType = deriveContextObjectType[Ctx, Query, Unit](identity) i get 'Field list is empty'

The only way I can get of this error is using IncludeMethods("users")

@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

@pahomovda which scala version are you using? If it's 2.11, it would be great if you can verify it with 2.12 as well.

@pahomovda

This comment has been minimized.

Copy link

commented Nov 26, 2016

@OlegIlyenko I am using scala 2.11, because play 2.5 doesnt support scala 2.12 for now as I know. I will try to move it to another project with scala 2.12, but if it will work with play 2.5 it would be great.

@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

I see. I noticed that for this particular issue 2.12 has very different behavior. To begin with, I could reproduce it very reliably. RC3 fixed the annotation discovery, but I myself still experienced flackyness with 2.11 (our backend is stuck on 2.11 as well).

@astorije

This comment has been minimized.

Copy link

commented Mar 6, 2017

I am running into this issue as well, using Sangria 1.0.0 and Scala 2.12.1:

$ sbt compile
[info] Set current project to accounts (in build file:[...]/accounts/backend/)
[info] Compiling 2 Scala sources to [...]/accounts/backend/target/scala-2.12/classes...
[error] [...]/accounts/backend/backend.scala:99: Field list is empty
[error]   val MutationType = deriveContextObjectType[AccountRepo, AccountRepo, Unit](identity)
[error]                                                                             ^
[error] one error found
[error] (compile:compileIncremental) Compilation failed
[error] Total time: 6 s, completed Mar 6, 2017 12:30:36 AM

(Yeah, I know, deriveContextObjectType[AccountRepo, AccountRepo, ...] is probably not ideal, but I'm still very new to all the things involved here and I'm still trying to figure this out 😅)

Same as OP, it used to work fine, then started to see this after I rebuilt from scratch and now it won't compile.
It's still hard for me to say if this is a bug on my end or not, so here is the exact code at this exact commit where I'm noticing this issue.


EDIT: As mentioned above, the following change worked around this issue:

-  val MutationType = deriveContextObjectType[AccountRepo, AccountRepo, Unit](identity)
+  val MutationType = deriveContextObjectType[AccountRepo, AccountRepo, Unit](
+    identity,
+    IncludeMethods("createTransaction", "updateTransaction", "deleteTransaction")
+  )

Thanks for mentioning this to I could link those things together. It will work for me for now but happy to help testing to bring @GraphQLField back if you want.

astorije added a commit to astorije/accounts that referenced this issue Mar 6, 2017
@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Mar 6, 2017

@astorije Thanks a lot for sharing this info! This is quite puzzling, macro behave in a quite unpredictable way when it comes to annotation discovery. I though that scala 2.12 is improved in this respect, but turns out that there are still some issues. I need investigate it further.

@OlegIlyenko OlegIlyenko reopened this Mar 6, 2017

@pahomovda

This comment has been minimized.

Copy link

commented Mar 6, 2017

interesting fact:
looks like annotation with parameters like @GraphQLTags(...) is never missed.

@mielientiev

This comment has been minimized.

Copy link

commented Aug 4, 2017

Have the same issue.
Scala 2.12.3. Sangria 1.2.2

@marioduarte

This comment has been minimized.

Copy link

commented Nov 2, 2017

Unfortunately this issue is still there with Scala 2.12.4 and Sangria 1.3.2 😞

@ghost

This comment has been minimized.

Copy link

commented Nov 22, 2017

Same here. Scala 2.12.4, sbt 1.0.3, and sangria 1.3.2

dotta added a commit to dotta/sangria-graphql-derivation-bug that referenced this issue Dec 24, 2017
dragos added a commit to dragos/sangria that referenced this issue Dec 29, 2017
Initialize symbols before checking their annotations.
The Scala compiler uses laziness extensively to deal with recursive types. Annotations are one instance where an uninitialized type will be incorrect, so we call `info` before checking method annotations inside macros. The proper method, initialize, is not available in the public API so I just called .info, which is what most places in the compiler do, anyway.

Fix sangria-graphql#172
@OlegIlyenko

This comment has been minimized.

Copy link
Member

commented Dec 29, 2017

Looks like #317 has a solution for this issue. Let's hope it will solve all of the edge-cases. If not, please feel free to reopen/comment on this issue (or create a new one) to narrow down the remaining edge-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.