Interfaces are not GraphQL compliant

This is how the GraphQL spec defines interfaces:

interface NamedEntity {
  name: String
}

type Person implements NamedEntity {
  name: String
  age: Int
}

Notice how the name field is repeated in the type implementing the interface. Not doing so is an error. In Dgraph, however, we omit the repeated field:

interface NamedEntity {
  name: String
}

type Person implements NamedEntity {
  age: Int
}

At first, this doesn’t come across as a major problem - I personally found it redundant to specify the field names twice. However, besides not being compliant, it causes another issue. Take the following example:

interface Fruit {
    id: ID!
    price: Int!
}

type Apple implements Fruit {
    id: ID!
    price: Int!
    color: String!
}

type Banana implements Fruit {
    id: ID!
    price: Int!
}

Notice how Banana doesn’t have any extra fields - the only difference is the type. This should be valid GraphQL, however, since we don’t repeat field names in the type with Dgraph, it errors out, because the type definition of Banana is now empty.

3 Likes

So basically you want to say that either we should enforce repetition of fields inside type to make it GraphQL compliant or we should allow for empty types if they implement an interface.

If you have to choose between the two option.

Go for the repetition route, because:

  1. Without repetition, documentation added to keys of an interface doesn’t show up for the type upon introspection!
  2. Also GraphQL Node Editor doesn’t work with Schema without repetition!
2 Likes

I too think we should aim for repetition of fields, since it makes our interfaces GraphQL compliant.

This should be a bug then. The idea behind not repeating field names was to avoid redundancy and make things easy for the user.

The schema that is finally generated from the input schema is guaranteed to be GraphQL compliant. Though you bring up an interesting point and we will look into it.

2 Likes

Also directives used to build the schema/resolvers. This could cause issues if different indexing or custom directives were applied between implementations.

2 Likes

I have found the bug, there seems to be an issue in parsing the empty type definition.

@minhaj empty types are invalid GraphQL, so I don’t think that is a bug. I had a look at the spec:

Quoting from here:

  1. An Object type must define one or more fields.

Quoting from here:

  1. An Interface type must define one or more fields.

There is an issue here to add support for empty types, but it has not been accepted yet.

@ajeet I am sorry for not elaborating on my point. It is true that an object must define one or more fields.
Actually, there is a lot of processing that needs to be done when push a GraphQL schema. We don’t want to allow replication of fields inside the type as said by Pawan so the Parser needs to be changed in a way that it should accept even empty type declaration in the initial step. Repopulation of the interface’s fields inside the implementing type is done internally and the schema is validated after that. So even if a user just defines an empty type that does not implement an interface, it is gonna result in error during the validation because it will just be empty after internal processing.

In other words… the types used by dgraph to generate the graphql endpoint may be non graphql spec compliant, but the endpoint generated itself will always be graphql spec compliant.

This may throw errors in an IDE but will produce a schema that is valid.

So currently what is the solution for empty types?
Should I copy the fields from the interface or maybe extract just one field and copy it to each implementation?
Is there an estimated time that this bug will be solved?

Thanks

@spinelsun you can create a dummy: Int field in each implementation. Since this is an optional field, you won’t have to change anything in your code.