Fragment spread on interface type with no implementers

[goto-bus-stop]

Hi! I think this may be a spec bug.

Say I have an interface with no implementers.

interface Intf {
  field: Int
}
type Query {
  intf: Intf
}

I can query this field and its subfields. It doesn't really make sense as intf can officially never return any concrete type but it does validate. So far so good.

query {
  intf { field }
}

However, if I put a fragment spread in there, with type condition Intf (i.e., the spread is useless because it's exactly the parent type), I believe the spec says that I should receive an error:

query {
  intf {
    ... on Intf { field }
  }
}

The Fragment Spread Is Possible rule dictates that this should check if the possible types of Intf and Intf should intersect. The possible types are defined as the set of types implementing Intf. This set is empty, so they don't intersect, so I believe the fragment spread should not be possible.

graphql-js accepts this query, because it first checks if typeCondition == parentType (source). graphql-go and graphql-java follow JS. In apollo-rs, we implemented the spec steps as written, as an intersection between GetPossibleTypes() (source), so apollo-rs rejects the query.

I think the graphql-js behaviour makes more sense, but as far as I can tell, it's not aligned with the spec. Should the spec be changed to fit, with an early bailout in the Fragment Spread Is Possible rule?

[andimarek]

A related question to this would be, if it is even allowed to have an Interface without implementation.

Afaik this question was not clearly answered so far.

I tend to think that it should not be allowed and the Schema you have posted should not be allowed in the first place.

[goto-bus-stop]

I agree, though that discussion is maybe too big in scope for this issue 😅 I'd like to clarify what the appropriate behaviour is for GQL as it is today, so I'm assuming here that we can define an interface without any implementations. If that changes, of course this whole thing would be moot from that spec version onward!

[andimarek]

I would argue that currently the behavior of the example presented here is undefined and we have to answer first if the Schema is actually valid.

As a data point that hints at that Interfaces must have at least one implementation, the spec says:

https://spec.graphql.org/draft/#sec-Interfaces.Result-Coercion

The interface type should have some way of determining which object a given result corresponds to. Once it has done so, the result coercion of the interface is the same as the result coercion of the object.

This reads for me as "every interface must have an object at execution time representing an actual implementation".

[goto-bus-stop]

Hmm. I think the definition of the Fragment Spread Is Possible validation itself is well-defined and unambiguous and it says that interfaces with no implementations are always an error. However I see your point that the schema that declares the interface, rather than the query, is undefined or at least a grey area. That logic can at least provide a justification for doing what graphql-js et al do despite it not actually written into the algorithm, because validating against a schema's type with undefined behaviour would also have an implementation-defined result.

I still think it'd be worthwhile to write the behaviour into the algorithm and thus have a specified way to deal with interfaces without implementations for the Fragment Spread Is Possible validation. I think officially disallowing interfaces without implementations will break existing schemas, while adjusting the Fragment Spread Is Possible algorithm by adding a "parent type equals type condition" check as the first step is forward-compatible and backward-compatible (it makes some implementations like apollo-rs non-compliant, but it doesn't break any queries that people are sending today).

[andimarek]

You are brining up great points and I agree the spec should be improved in one way or another.

I would recommend to bring this issue up in a GraphQL WG session to discuss further.

I suspect any kind of clarification needs to go through the proper process for spec changes because of the implications discussed here.

[benjie]

A related question to this would be, if it is even allowed to have an Interface without implementation.

This has been discussed a few times at the WG, and apparently this is something Facebook (IIRC?) use quite a lot and is desired as a schema evolution feature.

The Fragment Spread Is Possible rule dictates that this should check if the possible types of Intf and Intf should intersect. The possible types are defined as the set of types implementing Intf. This set is empty, so they don't intersect, so I believe the fragment spread should not be possible.

This does seem like it needs clarification in the spec one way or the other; I think changing the final line to Either {fragmentType} must be {parentType}, or {applicableTypes} must not be empty. should be sufficient; this would be a non-breaking change as it's more accepting than the previous spec text.

Trivia

Given this schema: ```graphql interface I { a: Int } type A implements I { a: Int } type B implements I { a: Int } type Query { q: I } ``` the following query should be valid: ```graphql { q { ... on A { ... on I { ... on B { a } } } } } ``` you'll notice that we can reason this will never match anything (nothing can be both A and B) but it's still valid.

[martinbonnin]

Another interesting case is interfaces with interfaces:

interface A {
  aField
}

interface B implements A {
  aField
  bField
}

type Query {
  b: B
}

query {
  b { 
    ... on A { aField } 
  }
}

I'm guessing it fails with graphql-js? If that's the case, it feels a bit inconsistent that there is a special case for an exact match vs matching a sub-interface. Both those cases are known to be always true.