-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Type reflections #473
Type reflections #473
Conversation
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.
@@ -8,10 +8,12 @@ module GraphQL | |||
# | |||
class Directive | |||
include GraphQL::Define::InstanceDefinable | |||
accepts_definitions :locations, :name, :description, :arguments, argument: GraphQL::Define::AssignArgument | |||
accepts_definitions :locations, :name, :description, :arguments, :default, argument: GraphQL::Define::AssignArgument |
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.
For the sake of readability, what do you think about calling this default_directive
?
It has better symmetry with the rest of these changes here and seeing things like default = false
in isolation feels a bit confusing.
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.
Oh yeah, that's much more consistent!
GraphQL::FLOAT_TYPE, | ||
GraphQL::ID_TYPE, | ||
] | ||
|
||
# By default, these are included in a schema printout |
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.
Huge ❤️ for making this change in the printer as well.
@@ -24,6 +34,24 @@ def initialize_copy(other) | |||
# @return [String, nil] a description for this type | |||
attr_accessor :description | |||
|
|||
# @return [Boolean] Is this type a predefined introspection type? | |||
def introspection? |
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.
We may be able to change this
graphql-ruby/lib/graphql/schema/validation.rb
Line 181 in 895d8b7
if type.name.start_with?('__') && INTROSPECTION_TYPES[type.name] != type |
introspection?
.
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'll give it a try
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.
👌 donezo
Thanks for taking a look at this! |
When metaprogramming a GraphQL schema, it comes in handy to check for special types: introspection types, built-in scalars and built-in relay types.
Now,
BaseType
responds to checks for those things.Additionally, built-in directives are flagged as such. (There's not really a way to add directives from user-space yet, but ... someday 😎 .)