When merging type with different set of interfaces, distinct interfaces are not always added.
berend opened this issue · 3 comments
Given the two schema A and B, where B has more interfaces on a type, the order of merging changes the outcome.
Example:
A
schema {
query: Query
}
type Query {
node(id: ID!): Node
Thing(id: ID!): Thing
}
interface Node {
id: ID!
}
type Thing implements Node {
id: ID!
foo: String!
}
B
schema {
query: Query
}
type Query {
node(id: ID!): Node
Thing(id: ID!): Thing
}
interface Node {
id: ID!
}
type Thing implements Meta & Node {
id: ID!
bar: String!
created_at: String!
}
interface Meta {
created_at: String!
}
If we now merge this with A
first like so:
combined, err = New([]*graphql.RemoteSchema{
{Schema: A, URL: "url1"},
{Schema: B, URL: "url2"},
})
then combined.schema.Types.[Thing].Interfaces
is ["Node", "Meta"]. If we merge with B
first
combined, err = New([]*graphql.RemoteSchema{
{Schema: B, URL: "url2"},
{Schema: A, URL: "url1"},
})
then combined.schema.Types.[Thing].Interfaces
is ["Node"].
This may cause issues, when doing fragments on the resulting type. It looks like, Schema A is overwriting Schema B here, instead I would expect a merge here, also order is not relevant.
I have to investigate a bit more, but overwrites should never occur, because:
- they have the same amount of interfaces (or fields,...) -> nothing to do
- they have additional ones -> should be added
- they have conflicting ones -> should return an error
I will try to fix that with another PR, but I am also wondering on your opinion on this one here, @AlecAivazis
Hey @berend! Thanks for bringing this up. I agree that we should never be overwriting interfaces and i think i agree with the three cases you outlined.
If you have the time to throw together a PR, id love to review it
Hi @AlecAivazis, just to let you know: I still plan to fix this, but christmas is the busiest time here, I have not yet found the time.
Thanks for opening @berend! This is fixed in v0.2.8