graphql-js: Querying same field w/ and w/o attributes for different union types results in error
Query:
{
curatedList {
...on Program {
title(lang: "fi")
}
...on Article {
title
}
}
}
Error:
Fields title conflict because they have differing arguments.
Should it really work like that, or is this a bug?
Article and Program are both members of union type Content
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Reactions: 7
- Comments: 29 (8 by maintainers)
Commits related to this issue
- [validation] Allow safe divergence As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as ... — committed to graphql/graphql-js by leebyron 9 years ago
- [validation] Allow safe divergence As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as ... — committed to graphql/graphql-js by leebyron 9 years ago
- [validation] Allow safe divergence As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as ... — committed to graphql/graphql-js by leebyron 9 years ago
- [validation] Allow safe divergence As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as ... — committed to graphql/graphql-js by leebyron 9 years ago
- [validation] Allow safe divergence As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as ... — committed to graphql/graphql-js by leebyron 9 years ago
This is so bad, I have several TS interfaces and I expect my results to match those interfaces, if I start aliasing the unions then they won’t match and I’m going to have to convert the results to match the TS interfaces.
I’m going to use just a plain
JSONResolverfor the conflicting fields. Graphql should support unions for scalars, and if that goes against the spec, then the spec is probably wrong and should be fixed. There should be at least a way to relax this or provide a solution that does not imply aliasing or having to interpret the results after in any way.Union types are ambiguous by definition, you either support them or you don’t.
Name one other type system that has a union type that requires fields of the unioned types to match…this was a nasty surprise and it makes me angry. And the fact that it was a deliberate decision even moreso.
In my case I had two types that unioned just fine until today, when I needed to make a field on one of them optional. Now I have to do extra work to de-alias the conflicting field in the query result 🙁
@schrockn-zz
Whichever one corresponds to the
__typenameof the resolved object…Flow and TS are perfectly capable of representing union types where the unioned objects have fields with the same name and different types:And as I mentioned in my above comment, I don’t know of any language whose type system is incapable of representing a union where field types mismatch.
Even if you’re not querying the
__typenamefield there would often be ways to do type refinements in Flow or TS with runtime checks, but if that proved difficult you would just solve it by including__typenamein the query. I fail to see how the different field types would actually cause problems.I think we could probably improve this if two fields are known to never “merge”. In this case, the two types are Object types and in no condition would both “title” fields be queried for the same object.
We should be a bit smarter about this (likely common) case
I don’t understand why querying for a field with different types for some members of a union should result in an error iff you include the
__typenamein each type’s fragment. There can be no ambiguity.@IvanGoncharov They do have predictable types depending on the parent type, so I do not really understand your statement. I will also add that the behavior was already changed for aliases to allow this very thing, so I do not see how this is any different.
@mkochendorfer as @IvanGoncharov said, this is correct by the spec. Your example is actually one of few remaining areas of the spec that rubs me the wrong way; I definitely see its utility in the context of implementing parsers in certain languages, but with only a cursory analysis I found the restriction unnecessarily, given that client limitations shouldn’t limit the abilities of the server (instead, they can just add aliases to their queries when they enter such a scenario, as all clients must do now).
I’ve currently got two projects I’m trying to champion for GraphQL (implementation of interfaces by interfaces and big TS changes), but as soon as one of these wraps up, I’m going to try to formalize a case for removing this restriction, and present it to the working group.
@mkochendorfer Response fields should have predictable type it should be the single type not the combination of
stringandComponentTwoBackground. Also implemented according to GraphQL Specification: https://graphql.github.io/graphql-spec/draft/#SameResponseShape()@leebyron So it appears this issue was addressed for aliases but never for actual type definitions. Consider this example schema:
With this schema the following query would be very legitimate:
However, this will always give you an error about the types of the background fields not matching . Any reason why this https://github.com/graphql/graphql-js/blob/master/src/validation/rules/OverlappingFieldsCanBeMerged.js#L603 type check cannot be moved up inside of the
if (!areMutuallyExclusive) {conditional?How is this still a problem 8 years later this is really frustrating
Ugly workaround: If you patch graphql and remove the rule everything works fine (except linter in explorer, that will still complain) and its a 1-liner. So maybe we can have an option for disabling that rule (or replace it with a less strict version)?
Yeah, I guess what I am fundamentally saying is that based on https://github.com/graphql/graphql-js/issues/53#issuecomment-121337250 and the resulting PR https://github.com/graphql/graphql-js/pull/229 it seems like the spec is already our of sync with this implementation. That original PR to address this issue seems like it would have worked for my case actually as it just exits immediately if the parent types differ. That code has since been changed causing my issue to resurface. https://graphql.github.io/graphql-spec/draft/#example-54e3d seems like a fundamentally wrong counter example based on all of the above, so if that means the spec needs to be updated to get this corrected, then I am all for it.
Is there some reason behind this restriction in the spec?
This just bit me. I designed an entire section of schema around a union thinking that query fragments would be sufficient to disambiguate the possible values of a field whose name is shared by all members of the union but whose return type differs. In my case the field types are different
enums, and of course GraphQL can’t union scalars orenumtypes, nor does it have a “constant” or “literal” type like Typescript, which will happily let you declaretype Foo = 'foo'ortype Foo = { name: 'Foo' }. I can’t use an interface because its type requirements are even more strict than a union.If you’re querying different fields of the unioned types, a given field isn’t guaranteed to be present in the response. So why does a field need to have a guaranteed type in the response either? Even if field types merge, they could have different semantic meaning that might be ambiguous if the query doesn’t include __typename or other distinguishing fields.
Running into this as well. Pretty major limitations for union types and interfaces. From my perspective, if we’ve used
resolveTypeto disambiguate the parent type and provide it the correct__typename, then I don’t see why any children properties of that top level type need to share properties…Dang, just stumbled upon this issue while trying to use fragments on a union-typed field with types that share a field name but each is its own type:
Any way to ignore this error?
Can you explain what part you do consider as shooting you into feet?
The precedence thing might be such, but I don’t think my original example with program and article title is that unreasonable.
If you wish to query for both you should alias at least one of the fields to be unambiguous.