-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hint that a field of an union or interface type will resolve only to types specified in the selection set #951
Comments
@n1ru4l If I understand you correctly, you're saying it would be nice to have an opt-in way of having GraphQL limit you to only returning types from an interface-returning field that are compatible with what the user has requested; e.g. if they request My first comment would be, in these days of the widespread use of My second would be that this would require some kind of "look ahead" where GraphQL determines which concrete types are queried on the selection set, and then feeds this information into the resolver so that the resolver knows only to return these types. GraphQL doesn't currently have anything like this in the spec (as far as I can recall), so that would be quite a divergence from the status quo, I think. I'd also argue that GraphQL resolves things layer by layer, so what is returned from a resolver should never be dependent on its selection set. For example if I issue your I think the best approach to this would be instead to make the requirement a field argument: An interesting topic, for sure 🤔 |
@benjie I guess Regarding the lookaheads: You mean in terms of the specification? To me this kind of fits into
People are already doing this with solutions like the following: https://engineering.zalando.com/posts/2021/03/optimize-graphql-server-with-lookaheads.html We have seen a lot of our clients do similar stuff. For union fields, it is also currently the only safe way to introduce new types to a union type if you do not "own" all the operations executed against your schema and can instruct your API consumers to consume GraphQL in a future-proof way on the client. It could also help in generating better types for the
This is also the pattern that the GitHub API follows for their |
Yes, in terms of the spec.
It would certainly go somewhere within
This is a really good argument. We say that you should always expect a new type to be added to a union, and you should handle that case (i.e. having a
I agree with this desire. It might be good to lay out explicitly the problems we're solving (like the one above regarding supporting adding types to unions in a non-breaking way) and then lay out what the possible solutions to it could be, and the tradeoffs of each one. For example, the argument approach: could that be supported by codegen if we were to add a special directive (or similar) to that argument or union telling the type system what it means? |
Problem: Future Proof Union TypesHandling fields of a union type in a future-proof way is hard to handle on the client-side. Depending on who is consuming the schema, e.g. a different team or external public users, there is a high probability that adding a new type to the union will eventually end up breaking clients. There has been an initiative for allowing to constraint union members to an interface. This is usually great when union members contain similar types, but does not address the issue when the union types are different from each other and have no shared interface or properties at all. type UserNotFoundError {
message: String!
}
type User {
id: ID!
}
union UserByIdResult = User | UserNotFoundError
type Query {
userById(id: ID): UserByIdResult!
} A common pattern for avoiding unexpected breakage of clients after adding a new type to a union is to use the operation document being executed within the field resolver implementation in order to restrict the type (entity) being resolved to only those specifically selected through the fragments and inline-fragments that are within the selection set of that field. It is currently impossible to hint to clients that a union field has implemented such behavior, which makes it hard for tools like GraphQL Code Generator to take this into account and generate more client-friendly typings. Problem: Relay
|
This is a great summary of the problem, great work in writing it up. However, as currently stated, I think your proposed solution breaks normalized caching (the same field with the same arguments on the same type may return different values in different queries because the value is influenced by the selection set - so future queries may remove/add things to the store in an unexpected/unpleasant way). This could be addressed by having normalised caching factor in this behavior, but that would be quite a significant divergence from the status quo. More importantly, however, I think that it either breaks field merging, or it does unexpected things with fragments. Take for example the following query: query Q {
currentUser {
...FragA
...FragB
}
}
fragment FragA on User {
pet { ... on Dog { name wagsTail } }
}
fragment fragB on User {
pet { ... on Cat { name numberOfLives } }
} Imaging pet is a Cat; then we'd expect FragA to resolve to We effectively merge first and have this resulting query: query Q {
currentUser {
pet {
... on Dog { name wagsTail }
... on Cat { name numberOfLives }
}
}
} But now, the client code that depends on FragA will get unexpected behavior because they'll have a cat object that's not the Dog (the only thing that they queried and expected) - they'd expect This is why I recommended solving the problem via a field argument ─ it solves both the problems I laid out here. However, it does not solve your code generation issue without some further thought. Perhaps going with something like I suggested before: { search(needle: $needle, only: [User, Comment]) { ... } } and applying a special directive to the |
Hello, I just ran into this myself and wonder what's the status of this problematic? What can we do to make some progress in the matter? Can I participate anyhow? I would really welcome this functionality.. Thanks 🙂 |
Note: this is a very early thought-dump
Given the following schema and operation:
The generated TypeScript type must be:
The
Query.search
resolver implementation could process the selection set and only resolve to types that are actually requested via a fragment/inline-fragment.Full runnable example over here: https://codesandbox.io/s/graphql-yoga-interface-field-only-resolve-selected-types-bzw7b6
It could be helpful if there was a way of hinting via the schema (and introspection) that a specific field of an interface or union type will only resolve to those types that are explicitly selected via fragment/inline-fragments on the selection set within the executed document. E.g. via a directive.
For the following operation:
instead, we can now generate
For the following operation:
instead, we can now generate
In addition, the GraphQL execution algorithm could take that instruction into account and raise an exception in case the resolved type would not match the selected types within the selection set.
So far I am unsure what should happen if you select an
interface
type on such a field. I guess tools should interpret this as "this can now resolve to all entities that implement the interface", which would be future-proof in case a new interface member was added.This approach makes sense for union types, where there are currently no shared fields.
It could furthermore help people build more resilient and future-proof GraphQL schemas.
Related issues:
The text was updated successfully, but these errors were encountered: