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

Feature Request: Derivable custom input objects #1055

Open
msterny opened this issue Apr 12, 2022 · 7 comments
Open

Feature Request: Derivable custom input objects #1055

msterny opened this issue Apr 12, 2022 · 7 comments
Assignees
Labels
enhancement Improvement of existing features or bugfix k::api Related to API (application interface)
Milestone

Comments

@msterny
Copy link

msterny commented Apr 12, 2022

Is your feature request related to a problem? Please describe.
I want to be able to define a custom GraphQL input object for a struct/enum that cannot support derive(GraphQLInputObject). A clear way of doing this would be to automatically define a conversion between a Juniper-supported GraphQLInputObject and a Rust enum (which can be used to represent a discriminant union).

An Example
Given an enum FooBar that we want to define as a GraphQL input type:

enum FooBar
{ 
   Foo(i32),
   Bar(String),
}

Since GraphQL doesn’t support input unions as of this writing, the GraphQL schema would need to look like this:

input FooBar 
{
   Foo: Int
   Bar: String
}

This means that we’d need to create a separate FooBarInputObject, and validate/map into FooBar after the input has been parsed.

#[derive(GraphQLInputObject)]
#[graphql(name = "FooBar")]
struct FooBarInputObject {
   foo: Option<i32>,
   bar: Option<String>
}
impl From<FooBarInputObject> for FooBar {
   fn from(f: FooBarInputObject) -> Self {
      match (f.foo, f.bar) {
         (Some(foo), None)) => FooBar::Foo(foo),
         (None, Some(bar)) => FooBar::Bar(bar),
         _ => panic!("Bad input")
      }
   }
}

This works well enough for a very simple case, but we run into issues with reusable and composite input objects. Consider the case where FooBar is part of a struct that we want to define as an input object:

#[derive(GraphQLInputObject)] <=== Fails to compile!
struct FooBarBaz {
   foo_bar: FooBar,
   baz: String
}

Since FooBar cannot be an input object, neither can FooBarBaz. This would require us to define another struct just to be able to map FooBar. And this issue gets worse every-time we create another higher-level struct - if we want to nest FooBarBaz in another level, we have to create a third new-type!

Describe the solution you'd like
It would be useful to separate a GraphQLInputObject’s schema representation from its form in Rust. One way to do this is to support custom conversions for input types.

We could create a juniper::ConvertInput trait that can enable custom type conversions.

trait ConvertInput {
   type Input: FromInputValue;
   
   fn from(in: Self::Input) -> Result<Self>;
}

impl <__S, T> juniper::FromInputValue<__S> for T 
where T: juniper::ConvertInput {
   fn from_input_value(v: &juniper::InputValue<__S>) -> Option<Self> {
      let parsed: <Self as ConvertInput>::Input = juniper::FromInputValue::from_input_value(v);
      <T as juniper::ConvertInput>::from(parsed).ok()
   }
} 
@msterny msterny added the enhancement Improvement of existing features or bugfix label Apr 12, 2022
@msterny msterny changed the title Derivable custom input objects Feature Request: Derivable custom input objects Apr 12, 2022
@tyranron
Copy link
Member

@ilslv what do you think about this? We have similar problems in our codebase.

@ilslv
Copy link
Member

ilslv commented Apr 13, 2022

@tyranron sounds like a great idea, until GraphQL supports input unions. I'm not sure about ConvertInput thought. We can just support #[derive(GraphQLInputObject)] on enums, maybe with additional required #[graphql(to_struct)] attribute or something like that to avoid breaking changes, once proper support for input unions lands. So once this feature will be available in the GraphQL we could just make this attribute non-required.

@tyranron
Copy link
Member

@ilslv yeah, I've also was thinking about just allowing #[derive(GraphQLInputObject)] on enums. Full separation seems to be a little bit overkill here.

@msterny what do you say? Will it be enough?

@msterny
Copy link
Author

msterny commented Apr 13, 2022

@ilslv Allowing enums would be enough! I'm curious how that would look - would the derive create an internal struct that exposes all the enum variants as Options?

@ilslv
Copy link
Member

ilslv commented Apr 13, 2022

@msterny actually I think we can achieve this without any intermediary structs, just using internal machinery.
@tyranron I gave a little bit more thought about additional required attribute to avoid future breakings I proposed earlier and I think it's not worth it. Because as far as I can tell input unions won't be standardized in the near future, but this required attribute will be annoying sometimes. And I guess that every spec update will cause some breaking changes.

@tyranron
Copy link
Member

@ilslv yep, you're right. Even if input unions will be introduced, it still won't interfere with deriving input objects on enums.

So let's add this ability for 0.16 release.

@tyranron tyranron added this to the 0.16.0 milestone Apr 14, 2022
@tyranron tyranron added the k::api Related to API (application interface) label Apr 14, 2022
@tyranron tyranron modified the milestones: 0.16.0, 0.17.0 Feb 28, 2023
@tyranron
Copy link
Member

Rescheduling this to 0.17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::api Related to API (application interface)
Projects
None yet
Development

No branches or pull requests

3 participants