-
Notifications
You must be signed in to change notification settings - Fork 228
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
[2021-03-04] Implement Oneof Input Objects in GraphQL.js #648
Comments
Hey @benjie, I've been following your Oneof RFC and some of the discussions around it in the GraphQL Working Group. I'm excited about the proposal and want to help move the RFC forward. I have experience with GraphQL and TypeScript, so I'd be interested in helping write an implementation for GraphQL.js. Did you already have an implementation in progress or should I just start from scratch? Also, from this this comment, it sounds like you're still working through the syntax—do you have any thoughts on which syntax would be best to implement? Please let me know how I can help! |
Hey @erikkessler1; that's great! There's a few open questions:
I think it's fine to drop field arguments ("Oneof Fields" in the spec edit) and go with just object types and input object types. I think it's also wise to simply use the directive syntax for now - at least then we can play with it :) There's quite a bit of pushback on using @OneOf with output types (due to interfaces and unions already existing) but I'd like to see it implemented and then we can turn it off if we must to get it merged. I don't have an in progress PR currently, I was planning to start work on it in February/March, got a few other things to clear off my plate first. |
Starting with input object types and object types using the directive syntax makes sense to me. I'll see what I can do working off of those assumptions! |
Hey @benjie, I've got some initial progress to share that I'd love to get your early feedback on. For consumability, I've broken my implementation into a few PRs on my fork of
(The combined diff is here) Let me know your thoughts or what next steps I should take. |
Excellent work; I didn't see anything in the changes that didn't align with my intent. I've not had time to fully review (I need to get back to doing the spec edits at some point!) but it's looking good to me 👍 |
Great, thank you for taking a look! Are there any other immediate action items you can think of for me? |
Nope; just try it out in a real schema and see how you get on! 🙌 |
any updates on this? :) we have a use case that could benefit from this solution |
The RFC has been updated to drop Oneof Fields, I still need to find some time to test @erikkessler1's work above though. Have you tried it @nazreen? |
So oneof arguments is still included? For Hot Chocolate we implemented oneof inputs but did not implement the arguments yet. I personally see value in having oneof arguments. What is the current sentiment around these? |
“Oneof fields” are fields that accept one argument. The general feeling is that the goal is easy to achieve by using a oneof input object as the type of the only argument, and that not doing this limits schema evolution options. I’ve thus removed them. The main reason to consider them is syntax; where does the directive/keyword/etc go? |
Thanks to your work @erikkessler1, we've moved |
Co-authored-by: Benjie Gillam <benjie@jemjie.com> Closes graphql/graphql-wg#648
Co-authored-by: Benjie Gillam <benjie@jemjie.com> Closes graphql/graphql-wg#648
RFC PR
Note: Action Item issues are reviewed and closed during Working Group
meetings.
The text was updated successfully, but these errors were encountered: