Render improvements #780
Render improvements #780
Conversation
|
|
||
| union :search_result do | ||
| types [:order, :category] | ||
| end |
There was a problem hiding this comment.
@binaryseed This PR started with the need to handle the above type. Master blows up because in SDL union Foo = Bar | Baz uses type references with a name. In the case of a macro based schema however we have identifiers. Turning an identifier into a name reliably requires access to the entire type set so that you can lookup the type by identifier, and then use its name.
There was a problem hiding this comment.
Yes, I ran into this a few times when building this out.. I tried hard to avoid needing the entire list of types when rendering because that makes it difficult to enable rendering just an individual type's SDL representation via inspect.
It seems like you've got that working out fine though since it's passing tests :)
There was a problem hiding this comment.
I'm seeing you handle rendering both Blueprint.TypeReference.Name and Blueprint.TypeReference.Identifier separately, nice
There was a problem hiding this comment.
Yeah what I did is basically fall back to doing what you had where we just camelize stuff and hope for the best if the type can't be found. This allows us to render simple fragments of a schema without needing the whole thing.
| @@ -173,7 +168,7 @@ defmodule SdlRenderTest do | |||
| with {:ok, %{input: doc}} <- Absinthe.Phase.Parse.run(sdl), | |||
| %Absinthe.Language.Document{definitions: [node]} <- doc, | |||
| blueprint = Absinthe.Blueprint.Draft.convert(node, doc) do | |||
| inspect(blueprint, pretty: true) | |||
| Inspect.inspect(blueprint, %Inspect.Opts{pretty: true}) | |||
There was a problem hiding this comment.
This change ensures that if the schema cannot be rendered due to throwing an exception, that exception remains unhandled so that a proper stacktrace can be accessed.
| @@ -72,7 +72,7 @@ defmodule Absinthe.Mixfile do | |||
| {:benchee, ">= 0.0.0", only: :dev}, | |||
| {:dialyxir, "~> 1.0.0-rc.6", only: [:dev], runtime: false}, | |||
| {:phoenix_pubsub, ">= 0.0.0", only: :test}, | |||
| {:mix_test_watch, "~> 0.4.1", only: [:test, :dev]} | |||
| {:mix_test_watch, "~> 0.4.1", only: [:test]} | |||
There was a problem hiding this comment.
Enabling this in :dev made it impossible to use recompile in iex -S mix within the absinthe project, since it would re-run the entire test suite each time you did it.
|
|
||
| # For macro schema | ||
| defp render(scalar) when is_atom(scalar) do | ||
| scalar |> to_string |> Macro.camelize() |
There was a problem hiding this comment.
This inference was unsafe, since types can be named anything, the only safe way is to look it up from the type list.
| end | ||
|
|
||
| defp render(nil, _) do | ||
| raise "got nil" |
There was a problem hiding this comment.
- handle render
nilcase better
There was a problem hiding this comment.
When did you encounter nil?
There was a problem hiding this comment.
I didn't, this was more make sure nil wasn't treated as a simple atom identifier and rendered as type Nil.
| defp render(%Blueprint.Schema.UnionTypeDefinition{} = union_type, type_definitions) do | ||
| types = | ||
| Enum.map(union_type.types, fn | ||
| identifier when is_atom(identifier) -> |
There was a problem hiding this comment.
We should do a pass in the absinthe code base and eliminate all bare atom identifiers within blueprints and standardize on %TypeReference.Name{} | %TypeReference.Identifier{}.
|
|
||
| union :search_result do | ||
| types [:order, :category] | ||
| end |
There was a problem hiding this comment.
Yes, I ran into this a few times when building this out.. I tried hard to avoid needing the entire list of types when rendering because that makes it difficult to enable rendering just an individual type's SDL representation via inspect.
It seems like you've got that working out fine though since it's passing tests :)
| @@ -544,6 +553,20 @@ defmodule Absinthe.Schema do | |||
| |> Enum.map(&lookup_directive(schema, &1)) | |||
| end | |||
|
|
|||
| def to_sdl(schema) do | |||
There was a problem hiding this comment.
This is a nice function to have!
We should use it in Mix.Tasks.Absinthe.Schema.Sdl as well
|
|
||
| union :search_result do | ||
| types [:order, :category] | ||
| end |
There was a problem hiding this comment.
I'm seeing you handle rendering both Blueprint.TypeReference.Name and Blueprint.TypeReference.Identifier separately, nice
| end | ||
|
|
||
| defp render(nil, _) do | ||
| raise "got nil" |
There was a problem hiding this comment.
When did you encounter nil?
| defp render(%Blueprint.Schema.UnionTypeDefinition{} = union_type, type_definitions) do | ||
| types = | ||
| Enum.map(union_type.types, fn | ||
| identifier when is_atom(identifier) -> |
No description provided.