Exception thrown when interfaces is null inside an object [0.12.0] #746
Comments
@hlship Right, but the spec has additional constraints based on the value of
https://facebook.github.io/graphql/June2018/#sec-Object
https://facebook.github.io/graphql/June2018/#sec-Scalar That's is why according to the spec
Because |
|
@hlship BTW. same principle applies to |
|
Interesting; that's easy to miss, as I would have thought that the schema for __Type would have capture it, but in this case, a careful reading of the prose overrides. Cool enough. I can update Lacinia to match that, I've already done |
@hlship Agree 👍 What if we change Introspection SDL like this: type __Type {
# ...
# should be non-null for OBJECT and null for others
interfaces: [__Type!]
}What do you think? |
|
That would be helpful for the next person who hits this, I suspect. It's interesting that this is exactly what unions are for, but that would make building and using the introspection schema much more complex. |
@hlship I think the interface would be a better match: interface __Type {
description: String
}
type __TypeObject implements __Type {
description: String
interfaces: [__TypeInteface!]!
}Because with unions this query is invalid: {
__type(name: "SomeObject") {
description
}
}And you need to have inline fragments for every subtype.
Not sure about that. Would be interesting to ask Lee about the motivation for this design decision. |
|
Yes, I’m not sure why I thought interface but typed union.
On Wed, Nov 21, 2018 at 4:32 PM Ivan Goncharov ***@***.***> wrote:
It's interesting that this is exactly what unions are for
I think the interface would be a better match:
interface __Type {
description: String
}
type __TypeObject implements __Type {
description: String
interfaces: [__TypeInteface!]!
}
Because with unions this query is invalid:
{
__type(name: "SomeObject") {
description
}
}
And you need to have inline fragments for every subtype.
using the introspection schema much more complex.
Not sure about that. Would be interesting to ask Lee about the motivation
for this design decision.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#746 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADNtLVDRBkvu3ys6PrSwoi5l-c3zmUqks5uxfCXgaJpZM4YsBjH>
.
--
Howard M. Lewis Ship
Senior Mobile Developer at Walmart Labs
(971) 678-5210
http://howardlewisship.com
@hlship
|
The schema introspection definitions (https://facebook.github.io/graphql/June2018/#sec-Schema-Introspection) indicate that the
interfacesfield of__Typeis a nullable list.Our implementation of GraphQL (Lacinia) does, in fact, return a null for objects that do not implement an interface, rather than an empty list.
This results in an exception in the data pane at GraphiQL startup.
even though the GraphQL introspection query (here formatted for readability) matches, AFAIK, the introspection schema:
You can see here in the code (buildClientSchema.js.flow, line 213):
... there's a check that interfaces are returned ... which is IMO not valid, as
interfacesis nullable and a null was returned.A quick scan of the source code shows that this may apply to
possibleTypes,enumValues,inputFields,fields(in__Type) and__Field.args.By contrast,
__Directive.args, which is obviously more recent, was added to the spec asargs: [__InputValue!]!.I'm looking into a workaround where we define these fields as non-null, to force an empty list to be emitted into the response; still either the spec should be updated, or GraphiQL should be updated to match the current spec.
The text was updated successfully, but these errors were encountered: