Skip to content
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

Support of the @OneOf Input Object feature #1062

Open
lucarin91 opened this issue May 14, 2022 · 5 comments
Open

Support of the @OneOf Input Object feature #1062

lucarin91 opened this issue May 14, 2022 · 5 comments
Labels
discussion enhancement Improvement of existing features or bugfix

Comments

@lucarin91
Copy link

Is there any plan for implementing the RFC: OneOf Input Objects? It is not a final draft but it is getting a lot of attention and other GraphQL libraries already started to implement that.

I am also willing to contribute to the feature if you think it will be useful.

@lucarin91 lucarin91 added the enhancement Improvement of existing features or bugfix label May 14, 2022
@tyranron
Copy link
Member

@lucarin91 need to think about this. I'm quite unsure about the RFC itself. It looks like copy-paster attempt from Protobuf, but "why the hell" when we have unions in GraphQL? Seems like input unions consisting of input objects (optionally, scalars too) is the thing fitting more naturally into GraphQL.

@lucarin91
Copy link
Author

@lucarin91 need to think about this. I'm quite unsure about the RFC itself. It looks like copy-paster attempt from Protobuf, but "why the hell" when we have unions in GraphQL? Seems like input unions consisting of input objects (optionally, scalars too) is the thing fitting more naturally into GraphQL.

Yeah, I agree it isn't the cleaner solution, but for my limited understanding, they came up with that after the better solution, i.e., the tagged types, was not accepted. The tagged types were a general way to have unions on inputs and outputs, but I think there was concern on the implementation side because it will greatly change the language.

I am interested in this feature in order to have an input object that will be parsed as a rust enum with struct variants. I managed to implement that as a plain GraphQL object but I will not be able to have validation. So the user could provide every variant at once.

Do you think it will be a complex feature to add the support of this @OneOf directive?

@ilslv
Copy link
Member

ilslv commented May 18, 2022

@tyranron isn't this request basically the same as #1055 but with added directive, that should be generated in the output (introspection or GraphQL API definition)?

@tyranron
Copy link
Member

@ilslv the goal is quite similar, but the solution is not the same. This one proposes extension of GraphQL (as language) trying to emulate InputUnion via @OneOf directive, exposing tagged type information on a GraphQL types level. While in #1055 we only trying to parse regular InputObjects as Rust enums. One can achieve the @OneOf behavior in the user code via #1055, but not on GraphQL types level (runtime error vs GraphQL typecheck error). The opposite (#1055 via @OneOf) seems also cannot be achieved in all cases, because @OneOf seems to not work with Scalars.

@ilslv
Copy link
Member

ilslv commented May 19, 2022

@tyranron

@OneOf seems to not work with Scalars.

Looks like Scalars are allowed: graphql/graphql-spec#825

Can this allow a field to accept both a scalar and an object?

Yes!

input FindUserBy @oneOf {
  id: ID
 organizationAndRegistrationNumber: OrganizationAndRegistrationNumberInput
}

input OrganizationAndRegistrationNumberInput {
  organizationId: ID!
  registrationNumber: Int!
}

type Query {
  findUser(by: FindUserBy!): User
}

Exposing @OneOf to generated GraphQL spec has 2 benefits:

  1. It works like documentation
  2. Downstream tools can have better support for it. Through I don't have any experience with other GraphQL client implementations and don't know, how useful it is.

To be fair, I'm not sure whether those 2 arguments are enough to expose @OneOf. I don't know how would transition from that directive to proper Input Unions would be handled, but it may cause some breaking changes. While not exposing @OneOf definitely shouldn't break anything. I quite like the idea of avoiding implementation of any RFC, that isn't in the standard yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement Improvement of existing features or bugfix
Projects
None yet
Development

No branches or pull requests

3 participants