-
Couldn't load subscription status.
- Fork 441
Make graphql_object macro supports deriving object field resolvers #652
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
Make graphql_object macro supports deriving object field resolvers #652
Conversation
My editor uses `rustfmt` to format on save, but `rustfmt` doesn't pickup edition specified in `Cargo.toml` properly.
Not sure how to add reviewers, would be great if anyone can help. Thanks!
^^ @LegNeato @jmpunkt @Victor-Savu
While I was working on the PR, I also noticed a few potential issues. But I am not sure who to talk to or maybe they are actually intended or work-in-progress features:
-
Looks like its a typo?
juniper/juniper_codegen/src/util/mod.rs
Line 649 in d13305f
pub struct GraphQLTypeDefiniton { -
Derive
GraphQLObjectmacro for struct with lifetime doesn't work. The following struct found in a test suite, which can reproduce the issue, but seems its not been used in any test case.
juniper/integration_tests/juniper_tests/src/codegen/derive_object.rs
Lines 63 to 66 in d13305f
#[derive(GraphQLObject, Debug, PartialEq)]struct WithLifetime<'a> {regular_field: &'a i32,} -
Derive
GraphQLObjectmacro requiresscalarattribute, otherwise generic__Swill be used instead of the defaultDefaultScalarValue. For instance
#[derive(GraphQLObject)] struct Object { value: i32, }
should be the same as
#[derive(GraphQLObject)] #[graphql(scalar = juniper::DefaultScalarValue)] // <---- here struct Object { value: i32, }
- You can fix the typo.
- Do you know why this does not work? Is the lifetime parameter missing at the generics definition?
- There is an issue related to this (#[juniper::graphql_object] != #[derive(juniper::GraphQLObject)] #580 ). Not sure how to deal with this, I would ignore it for now. A workaround for this is to specify
#[graphql(Scalar = juniper::DefaultScalarValue)]
Thanks @jmpunkt !
Re 2:
Given the following code:
mod test { #[derive(juniper::GraphQLObject)] #[graphql(scalar = juniper::DefaultScalarValue)] struct WithLifetime<'a> { value: &'a str, } struct Query; #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)] impl<'a> Query { fn with_lifetime(&self) -> WithLifetime<'a> { WithLifetime { value: "blub" } } } }
Here is the compile error, which I think it is related to the async resolver (but can be wrong):
error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
--> integration_tests/juniper_tests/src/lib.rs:20:5
|
20 | #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first, the lifetime cannot outlive the lifetime `'a` as defined on the impl at 21:10...
--> integration_tests/juniper_tests/src/lib.rs:21:10
|
21 | impl<'a> Query {
| ^^
note: ...so that the expression is assignable
--> integration_tests/juniper_tests/src/lib.rs:20:5
|
20 | #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: expected `foo::WithLifetime<'_>`
found `foo::WithLifetime<'a>`
note: but, the lifetime must be valid for the lifetime `'b` as defined on the method body at 20:5...
--> integration_tests/juniper_tests/src/lib.rs:20:5
|
20 | #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...so that the expression is assignable
--> integration_tests/juniper_tests/src/lib.rs:20:5
|
20 | #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: expected `std::pin::Pin<std::boxed::Box<(dyn core::future::future::Future<Output = std::result::Result<juniper::value::Value, juniper::executor::FieldError>> + std::marker::Send + 'b)>>`
found `std::pin::Pin<std::boxed::Box<dyn core::future::future::Future<Output = std::result::Result<juniper::value::Value, juniper::executor::FieldError>> + std::marker::Send>>`
Here is the generated GraphQLTypeAsync for Query:
impl<'a> juniper::GraphQLTypeAsync<juniper::DefaultScalarValue> for Query where juniper::DefaultScalarValue: Send + Sync, Self: Send + Sync, { fn resolve_field_async<'b>( &'b self, info: &'b Self::TypeInfo, field: &'b str, args: &'b juniper::Arguments<juniper::DefaultScalarValue>, executor: &'b juniper::Executor<Self::Context, juniper::DefaultScalarValue>, ) -> juniper::BoxFuture<'b, juniper::ExecutionResult<juniper::DefaultScalarValue>> where juniper::DefaultScalarValue: Send + Sync, { use futures::future; use juniper::GraphQLType; match field { "withLifetime" => { let res: WithLifetime<'a> = (|| WithLifetime { value: "blub" })(); let res2 = juniper::IntoResolvable::into(res, executor.context()); let f = async move { match res2 { Ok(Some((ctx, r))) => { let sub = executor.replaced_context(ctx); sub.resolve_with_ctx_async(&(), &r).await } Ok(None) => Ok(juniper::Value::null()), Err(e) => Err(e), } }; use futures::future; future::FutureExt::boxed(f) } _ => { { ::std::rt::begin_panic_fmt(&::core::fmt::Arguments::new_v1( &["Field ", " not found on type "], &match ( &field, &<Self as juniper::GraphQLType<juniper::DefaultScalarValue>>::name( info, ), ) { (arg0, arg1) => [ ::core::fmt::ArgumentV1::new(arg0, ::core::fmt::Display::fmt), ::core::fmt::ArgumentV1::new(arg1, ::core::fmt::Debug::fmt), ], }, )) }; } } } }
While I like the idea of this in general, I think it massively complicates the UX for folks to save on some typing. This reminds me a lot of Rust accessors and transparent newtypes. Rust itself requires boilerplate (self.0) but people who mind the repetition build a layer on top with proc macros and there are multiple libraries that do it based on various tradeoffs. I always assumed people would just use the proc macro with a bit of boilerplate or use juniper_codegen to make their own macros with their preferred tradeoffs if this was an issue.
Personally, I like how simple and explicit the UX is right now--either graphql projection is trivial and the derive suffices or it is complex and everything is right there in the impl.
I'm happy to entertain a patch if we can keep this simple and explicit. I'm not sure we can get there with rust proc macros as-is though 🤷
@LegNeato Thanks for your reply, I understand your concern and agree that Juniper should not be opinionated.
However, since juniper_codegen already achieved a great effort on collecting necessary information to build GraphQL schema (such as fields for object / input types, field resolvers, etc), is it possible to expose these collected information to the user instead of generating the GraphQLType directly, so that user can use these information in their own macro to generate GraphQLType.
For example juniper can generate GraphQLTypeInfo that holds such information, similar to what this PR does. Then I can build my own graphql_object macro with the functionalities I need very easily.
Let me know what you think. Thanks!
smmoosavi
commented
Sep 12, 2020
can we have multiple derive_fields? it would be awesome if we can.
file1:
#[derive(GraphQLObjectInfo)] #[graphql(scalar = DefaultScalarValue)] struct Obj { regular_field: bool, } #[juniper::graphql_object(derive_fields)] impl Obj { fn custom_field_resolve() -> bool { true } }
file2:
#[juniper::graphql_object(derive_fields)] impl Obj { fn other_field_resolve() -> bool { true } }
4bb3114 to
33db991
Compare
37a5ec0 to
6036544
Compare
Uh oh!
There was an error while loading. Please reload this page.
This PR enhance the
graphql_objectmacro to optionally derive field resolvers for fields defined in the object type struct.I am pretty new to rust, please let me know if I have done anything incorrect. And I am not a native speaker of english, feel free to correct the wording of my comments. Cheers!
Related: #553