Repository navigation
Polymorphic input types #238
Description
Activity
I do understand your frustration.
GraphQLite uses webonyx/graphql-php under the hood for schema generation and GraphQL parsing.
I'm all in favor of implementing polymorphic input types but we need first to have it supported by webonyx/graphql-php. So I think you should first open an issue with them.Once it is supported in webonyx/graphql-php, it to GraphQLite should be quite easy.
The issue has been filed with webonyx now.
Been 4 years since this was posted so I'd be keen to see if there's any updates to this ? Been a good point of frustration for me aswell. Currently trying to make some form of workaround using a custom RootTypeMapper.
@noknokcody submit a PR with webonyx, then we can get a PR merged here as well.
Reacted by Cody ReesI'll share some thoughts on designs that, in my opinion, make sense, and ones that don't:
I wrote an entire chapter on different designs and then realized one important limitations that, fortunately, makes it easier for GraphQLite :) Although all of the fields in a
@oneOfinput must be declared nullable, none of them can actually acceptnull. This is counterintuitive and doesn't make any sense to me, but it allows the implementation to be simpler and not require #726 first.I'm thinking something like this:
class UserController { #[Query] public function find(FindUserBy $by): User { $emailProvided = $by->email !== null; } } #[Input] #[OneOf] // or - #[Input(oneOf: true)] // or without #[Input], just the #[OneOf] class FindUserBy { public function __construct() { #[Field] public readonly ?string $email = null, #[Field] public readonly ?string $phone = null, } }
Which should be pretty straightforward in terms of the implementation, and is somewhat type-safe right away. It's also how some other implementations did it: https://chillicream.com/blog/2022/01/13/hot-chocolate-12-5#oneof-input-objects
It still got some issues I don't like:
- technically, both
$emailand$phonecould be filled (for example, when testing the controller directly in a mocked test, not through GQL) - it's not possible to "match" based on which input was provided. That is, you have to check each field one by one, until you get to the one you want. In an example above, this is trivial, but if there are 10 fields it starts to get tricky
An alternative approach solving those issues would be something along the lines of:
class UserController { #[Query] public function find(FindUserBy $by): User { $emailProvided = $by->email !== null; } } #[OneOf] interface FindUserBy {} #[OneOf] class FindUserByEmail implements FindUserBy { public function __construct( #[Field] public readonly string $email, ) {} } #[OneOf] class FindUserByPhone implements FindUserBy { public function __construct( #[Field] public readonly string $phone, ) {} }
But this is much worse in terms of the implementation, requires a lot of limitations on how the class is built (exactly one implemented interface, exactly one field, a class per each field) and is quite verbose. While I like this more from a type-safety standpoint, I don't think this is the way to go.
If anyone's got any ideas they're welcomed :) So far I believe only the first design is a viable option, and I can't think of any other alternative to it.
- technically, both
After some additional review, I agree @oprypkhantc. There really isn't any other reasonable design.
As for the attribute, I maintain that keeping the number of attributes more limited in scope is a better design choice. There is also a direct dependency between
@oneOfand aninput. So, having a different attribute, that could potentially be misused, only leads to confusion.#[Input(oneOf: true)] // Let's use an argument
- technically, both $email and $phone could be filled (for example, when testing the controller directly in a mocked test, not through GQL)
This is a user-land concern IMO. unless we want to ship a base
OneOfclass that must be extended, and that's the basis for the implementation. That also could resolve the following issue.- it's not possible to "match" based on which input was provided. That is, you have to check each field one by one, until you get to the one you want. In an example above, this is trivial, but if there are 10 fields it starts to get tricky
Aside from some base class design, we could should ship some helper utilities around this - like a trait (sigh). I agree the DX is sub-optimal if you have to check all these fields. This is also similar to the issues from #726.
To be clear though, I dislike the idea of using inheritance. This has been avoided in GraphQLite, to date, and I think we should make all efforts to maintain that. It allows for the lib implementation to be much more flexible.
Reacted by Oleksandr Prypkhan
I'd like to go ahead and present this RFC for implementation, possibly early as it looks to have full support. Or, at the least, I'd like to get the discussion and ideas flowing for implementation in the future.
https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md
This is particularly of interest in cases where you have an update mutation that has optional relational inputs. In this case, you have to support creating and updating for the relation. It's also the case for one:many relationships, if you wish to update relations within a parent mutation.
There are a number of other use cases that are also important. The above use case is one that we're finding to be very common and a continual frustration point with the API/schema.