-
-
Notifications
You must be signed in to change notification settings - Fork 249
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 directive support #225
Merged
ghostdogpr
merged 7 commits into
ghostdogpr:master
from
paulpdaniels:add-directive-support
Feb 22, 2020
Merged
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b4079ac
Add basic directive support to types
paulpdaniels 7bc9b78
Add directive rendering
paulpdaniels f516108
Add apollo caching to test directives
paulpdaniels 9c727a1
Fix rendering spec
paulpdaniels 2c5ac56
Fix formatting
paulpdaniels 8218d59
Address feedback, fix render ordering, add docs
paulpdaniels 3b90b17
Fix doc typo
paulpdaniels File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,12 @@ | ||
package caliban.execution | ||
|
||
import caliban.introspection.adt.__Type | ||
import caliban.parsing.adt.Directive | ||
|
||
case class FieldInfo(fieldName: String, path: List[Either[String, Int]], parentType: Option[__Type], returnType: __Type) | ||
case class FieldInfo( | ||
fieldName: String, | ||
path: List[Either[String, Int]], | ||
parentType: Option[__Type], | ||
returnType: __Type, | ||
directives: List[Directive] = Nil | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
package caliban.introspection.adt | ||
|
||
import caliban.parsing.adt.Directive | ||
|
||
case class __Field( | ||
name: String, | ||
description: Option[String], | ||
args: List[__InputValue], | ||
`type`: () => __Type, | ||
isDeprecated: Boolean = false, | ||
deprecationReason: Option[String] = None | ||
deprecationReason: Option[String] = None, | ||
directives: Option[List[Directive]] = None | ||
) |
10 changes: 9 additions & 1 deletion
10
core/src/main/scala/caliban/introspection/adt/__InputValue.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,11 @@ | ||
package caliban.introspection.adt | ||
|
||
case class __InputValue(name: String, description: Option[String], `type`: () => __Type, defaultValue: Option[String]) | ||
import caliban.parsing.adt.Directive | ||
|
||
case class __InputValue( | ||
name: String, | ||
description: Option[String], | ||
`type`: () => __Type, | ||
defaultValue: Option[String], | ||
directives: Option[List[Directive]] = None | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not render it here than throwing an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking the spec after you mentioned this I realized there are a couple more things to be mindful of: https://spec.graphql.org/June2018/#sec-Type-System.Directives
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it would be possible to resolve these at compile time, so it probably would have to be a runtime error, but the question is how should we handle it? Agree that throwing isn't the best option, should we have two rendering methods? one that simply suppresses errors and one that returns an
IO[ValidationError, String]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit similar to issue #170 where we lack a way to report something abnormal within the schema. Ideally we should fail at the point of building the schema rather than later during query execution or rendering like here.
It bothers me to make
graphQL(...)
return an effect because attaching the wrappers with@@
wouldn't be as smooth as it is now, but how about making.interpreter
return an effect? That way we can build the schema easily, but when we turn it into an interpreter (which we do only once), we verify that the schema is valid and fail if it's not. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw if we go for that solution, we can do it in a later PR and just remove the check for now (so that you don't need to implement the whole thing to get this PR merged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense, since it is the "effect" of becoming an interpretable schema.
Ok I'm fixing so that it will just remove the arguments if they aren't valid and then we can fix it with the change you mentioned above.