-
Notifications
You must be signed in to change notification settings - Fork 529
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
Conform built in types to spec #1017
Conform built in types to spec #1017
Conversation
See https://spec.graphql.org/draft/#sec-Schema-Introspection `Absinthe.Introspection.Kind.kind` now returns an atom. It wasn't called anywhere except from introspection but it is a breaking change.
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.
Thanks, this is a good improvement!
@@ -8,19 +8,20 @@ defmodule Absinthe.Introspection.Kind do | |||
__MODULE__ | |||
|> Module.split() | |||
|> List.last() | |||
|> Absinthe.Introspection.Kind.upcase() | |||
|> Absinthe.Introspection.Kind.downcase() |
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.
Given this is enumerated now, and not just a string, I wonder if this can be simplified (ie: not use all that module / string manipulation)
It seems like the goal is just to have a function kind
that returns an atom... We could get rid of the macro altogether:
defmodule Absinthe.Type.Object do
@behaviour Absinthe.Introspection.Kind
def kind, do: :object
end
defmodule Absinthe.Introspection.Kind do
@type introspection_kind :: :enum | :object # ...
@callback kind() :: introspection_kind()
end
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 I'll do this separately...
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.
Actually while making this change, I found some further issues with our __TypeKind
!
Further corrections here: #1019
+1
to being explicit!
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.
Thx, good catch!
Changes the built in types to conform to the spec, see https://spec.graphql.org/draft/#sec-Schema-Introspection
Most notably adds the
__TypeKind
enum.Absinthe.Introspection.Kind.kind
now returns an atom for the__TypeKind
enum resolver.