swift-graphql
swift-graphql copied to clipboard
Interfaces shouldn't require each interface implementer to be present on selection building
Describe the bug
BrainTree overloads their 'node' interface to cover basically everything under the sun. And going through the node interface is the only way to query for updates on those implementers.
Is this the way?
Here is a link to their docs:
https://braintree.gitbook.io/in-store/guides/making-a-transaction#checking-the-reader-charge-status
Steps to Reproduce
Create an API with a node interface. Then build several objects that implement that interface. Then add a query through the node interface. Now build a query for one of the node interfaces.
Expected behavior
All arguments should be optional, with only one required
I expect that there is probably a better way to do this but I couldn't figure it out from the docs and my laptop is struggling with the generated api.
Running into this myself right now. Curious to know if this is, indeed the way we're supposed to be handling interfaces?
Seems to be the case - if so, maybe a documentation win to be had here.
Hey π ,
Frankly, i am also aware of this problem, but haven't gotten around to making meaningful progress. Do you think if we only made all arguments optional that that would be enough? If we do that, are there any issues that you see coming up?
From the top of my head, how would you want to handle items that are not of a handled interface; for example, say you want to list all nodes, but you only handle Customer
model.. if there's a Dispute
in the returned list, do you think the library should return nil
or would it also be okay if it just threw an error?
First off - appreciate the work you're doing @maticzav - this framework is a real joy to use.
Secondly, I won't pretend to understand the range of use cases that you have to cover but maybe my thoughts can grease the proverbial wheels.
The ethos behind swift-graphql as I understand it is - "What if you could use GraphQL but it felt Swift-like." A sentiment I greatly appreciate!
Thinking about how we would solve this as Swift developers in a world where GraphQL didn't have a say - I think most of us would reach for optionals or overloads (overloads would be super messy for the Braintree API @BerkeleyTrue is trying to consume ...) to handle something like this.
...If there's a Dispute in the returned list, do you think the library should return nil or would it also be okay if it just threw an error?
Optional with maybe silent warning feels right here π€ In both Berkley's case and mine - we've been "forced" to create mappers for types we have no desire to parse and consume. I'm not as experienced with GraphQL but that seems counter to what they're going for as well when thinking about API Contracts. Seems like they're all about "Define and consume only what you need."
Optional feels "right" and clear to me.
Best of luck!
@thingdeux just chiming in as I'm cleaning up issues. I need to properly review this to understand the issue more completely, but I just wanted to ensure its being looked into and not forgotten π
ββ
@thingdeux / @BerkeleyTrue / @maticzav
I just want to ensure Iβm understanding this more clearly. Would you mind clarifying whether Iβm not he right track or not?
Selection.Node<ChargeContext> { b in
let context: ChargeContext? = try b.on(
// all arguments are non-optional,
// but perhaps we simply donβt care about most of them
)
}
So essentially the optional
solution mentioned above would allow you to only include the arguments you care about?
@maticzav how would this affect the resulting query/selection? Perhaps its a non-issue? If so, how easy of an update is this do you think? Perhaps we could schedule it as a part of the v5 updates?
Thanks!
@shaps80 the arguments here tell the selection for each object in the schema that conforms to the interface (i.e. they are not mutation or query arguments).
The problem as mentioned above is finding a meaningful default that works in most cases when one of the interfaces wasn't handled.
Perhaps GraphQL Spec actually already prescribes the way to handle it, we should just find it.
Does that make sense?
@shaps80 the arguments here tell the selection for each object in the schema that conforms to the interface (i.e. they are not mutation or query arguments).
The problem as mentioned above is finding a meaningful default that works in most cases when one of the interfaces wasn't handled.
Perhaps GraphQL Spec actually already prescribes the way to handle it, we should just find it.
Does that make sense?
Not 100% but I get the gist I think. I think you're right, its a good for us to read the spec and see how this should be handled, def feels like something where we likely don't need to re-invent the wheel π