juniper
juniper copied to clipboard
Automatic getters for `#[graphql_object]`
Is your feature request related to a problem? Please describe. I'm always frustrated when I need to replace:
#[derive(juniper::GraphQLObject)]
struct Foo {
name: String
}
by
struct Foo {
name: String
}
#[juniper::graphql_object]
impl Foo {
fn computed(&self) -> String {
...
}
}
because I lose the nice default field accessor for name and I have to manually add it to the impl:
#[juniper::graphql_object]
impl Foo {
...
fn name(&self) -> &String {
&self.name
}
}
Describe the solution you'd like
I'd like for #[derive(juniper::GraphQLObject)] and #[juniper::graphql_object] to work together so I don't have to add the dummy accessors for every field in the impl block.
I would imagine it is complicated to achieve this because both macros currently generate the same impl block underneath.
Describe alternatives you've considered I couldn't come up with any alternative to this :crying_cat_face:
Additional context This is extra painful when wrapping REST interfaces which have many simple fields (like strings) and a few complex fields (references to other resources).
P.S.: juniper is such a great crate! many thanks to all the contributors! :100:
Could have sworn we had an enhancement request for this alrteady but can't find it. I agree this would be very nice to have.
As a workaround you can make your own macro or derive that spits out a #[juniper::graphql_object] for you with the boilerplate included.
@LegNeato Thanks for the quick reply! I quickly went through all of the open items before posting just because I was really expecting to see this issue :) I hope I didn't miss anything and if I did I'm sorry.
I will have a look at how to achieve that with my own macro for now. Would it be useful to leave this issue open and see if anyone else may want this feature?
Yep!
I tried looking into rolling my own macro for generating the necessary fields and found a few things that I wanted to share in case you or someone else has an idea for how to best approach this.
The problem I ran into was that I could not solve this problem with a derive macro. In order to have since derive macros only apply to either the struct item or to the impl item so it would be impossible to get information about both without using a function-like procedural macro. I opened a draft PR #592 with an implementation that demonstrates the results.
@LegNeato The PR is just to get an idea of whether you and the other juniper maintainers would be open to this direction.
I had an idea how to just make #[derive(GraphQLObject)] and #[graphql_object]) work together, but the implementation depends on rust-lang/rfcs#1210 (or rather on #![feature(specialization)] in nightly).
Here is the general design: We can have two intermediate traits:
GraphQLDataderrived from thestruct/enumby#[derive(GraphQLObject)]; andGraphQLImplthe implementation of which is generated from theimplblock by#[graphql_object].
The two traits would each specify similar methods as the GraphQLType. We can then have a default implementations of GraphQLType based on objects that implement GraphQLData and a specialization for those that implement GraphQLImpl. For coherence reasons, we need GraphQLImpl to require GraphQLData:
trait GraphQLImpl : GraphQLData {
...
}
The downside of the design in my comment above is that impl GraphQLData becomes necessary even for cases where users don't want to expose some (or any) of the fields in a struct. Also, it is needed for rust enums as well. A hopefully convenient way to solve this is to provide default implementations for the methods in GraphQLData such that the user would only need to write:
struct Y { y: usize } // don't want to expose Y::y
impl GraphQLData for MyType {}
to satisfy the condition and to hide all the members in MyType. This option is shown in the playground example as well.
Another possibility is to use handler functions. The user defines structures which contain data and handler fields. For each data fields code is generated. A Handler field references a function, similar to Serde. However, this could require to remove non-async code first, because we can not distinguish between async and non-async functions.
#[derive(juniper::GraphQLObject)]
struct Foo {
name: String
#[graphql(Handler = foo::computed)]
computed: String,
}
mod foo {
fn computed(&self) -> String {
...
}
}
Maybe this approach fits better, but I am not sure.
@jmpunkt This sounds great as well! The design space for this problem seems bountiful.
However, this could require to remove non-async code first, because we can not distinguish between async and non-async functions.
I think if we can tolerate a bit more typing, we can specify when a handler is synchronous. The default could be async:
#[derive(juniper::GraphQLObject)]
struct Foo {
name: String
#[graphql(Handler = foo::computed, synchronous = true)]
computed: String,
#[graphql(Handler = foo::requested)]
requested: String,
}
mod foo {
fn computed(&self) -> String {
...
}
async fn requested(&self) -> String {
...
}
}
A current implementation which compiles (nothing tested) looks like the following:
#[derive(GraphQLObject, Debug, PartialEq)]
pub struct HandlerFieldObj {
regular_field: bool,
#[graphql(Handler = handler_field_obj::skipped, noasync)]
skipped: i32,
#[graphql(Handler = handler_field_obj::skipped_result, noasync)]
skipped_result: i32,
#[graphql(Handler = handler_field_obj::skipped_async)]
skipped_async: i32,
#[graphql(Handler = handler_field_obj::skipped_async_result)]
skipped_async_result: i32,
}
mod handler_field_obj {
pub fn skipped(obj: &super::HandlerFieldObj, ctx: &()) -> i32 {
0
}
pub fn skipped_result(obj: &super::HandlerFieldObj, ctx: &()) -> juniper::FieldResult<i32> {
Ok(0)
}
pub async fn skipped_async(obj: &super::HandlerFieldObj, ctx: &()) -> i32 {
0
}
pub async fn skipped_async_result(
obj: &super::HandlerFieldObj,
ctx: &(),
) -> juniper::FieldResult<i32> {
Ok(0)
}
}
A main different to the examples before, is that it is required to have a context and to use the type of the object rather than self.
To fix this issue, an impl macro for this special case is required. This impl macro provides self binding and context handling. Therefore, the mod block would look very similar to an impl block. The macro would transform the function into same format as the example, but would use a similar input as the impl macro.
@jmpunkt I'm impressed and surprised. Was this possible all along? I thought I went over the book with a comb and couldn't find the graphql attribute macro on struct fields taking a Handler. Is this documented somewhere and I totally missed it?
No it is not possible with the code on the master branch. Due to the structure of Juniper, it does not require that much changes to support this behavior.
The sentence
A current implementation which compiles (nothing tested) looks like the following
refers to my testing branch.
I am slightly disagree the proposed approach above. For instance, I am using diesel to query the database:
#[derive(Debug, Clone, PartialEq, Queryable, Identifiable)]
pub struct User {
pub id: i32,
pub first_name: String,
pub last_name: String,
// pub full_name: String,
}
#[juniper::graphql_object]
impl User {
pub fn full_name(&self) -> String {
format!("{} {}", self.first_name, self.last_name)
}
}
In the above example, I won't want to put the full_name field in User struct as there is no such column in the users table. As a workaround, I have to define another struct that doesn't contain the full_name field for diesel, which really just delegating the boilerplates...
I did not think of that. Since the original idea was to reduce the boilerplate, what about this?
#[derive(Debug, Clone, PartialEq, Queryable, Identifiable)]
pub struct User {
pub id: i32,
pub first_name: String,
pub last_name: String,
}
#[juniper::graphql_object(id: i32, first_name: String, last_name: String)]
impl User {
pub fn full_name(&self) -> String {
format!("{} {}", self.first_name, self.last_name)
}
}
We define trivial fields inside graphql_object. The macro provides trivial implementations for the specified fields.
Explicitly defining trivial fields also works.
Or I am not sure if it's possible that the macro provides trivial implementations for all fields in the User struct by default (like #[derive( GraphQLObject)]), unless a field has #[graphql(skip)]. And methods in impl User are treated as field resolvers, which can be additional to (or overriding) the provided trivial implementations.
I had been using type-graphql with TypeScript, which has very similar experience like this.
It would be awesome if we can combine with this feature request (#647).
The overall problem with this feature is that a macro can only see the underlying AST, e.g., impl macros can only see the impl block. There is now way for the #[juniper::graphql_object] to know what fields, e.g., User has.
My last statement is maybe not 100% true. We could get information from structs if we generate code which provide these information. We define a trait JuniperObjectInfo which can hold field information of a struct. The trait is implemented by the proc macro GraphQLObjectInfo. Instead of implementing the usual Juniper logic, only the trait with the necessary field information is generated. Finally we specify that #[graphql_object] must use the previous defined information in JuniperObjectInfo. During the code generation the following line is generated <#obj as JuniperObjectInfo>::fields() which returns the previously defined fields. The proc marco now has all necessary information to generate code which includes fields not defined inside the impl block. If #[graphql(partial)] is not specified, then only fields listed inside the impl block are used.
Notice that #[derive(GraphQLObjectInfo)] has the same attributes as #[derive(GraphQLObject)]. The only difference between these macros is the generated code.
#[derive(juniper::GraphQLObjectInfo)]
struct Obj {
field: String,
}
impl juniper::JuniperObjectInfo for Obj {
fn fields() -> &'static [Field] {
&[ /* generated by macro */ ]
}
}
#[juniper::graphql_object]
#[grapqhl(partial)]
impl User {
pub fn full_name(&self) -> String {
format!("{} {}", self.first_name, self.last_name)
}
}
Not sure if this works.
@zhenwenc Thank you for your interest in this issue! I was wondering if you had a look at my PR #592 for an alternative syntax (admittedly, a bit of a stretch as far as syntactical changes go). Thanks!
@Victor-Savu Thanks! I did had a look the PR, its indeed a very nice idea! But I think it requires a bit too much changes on the current API. I do like the current way of defining field resolvers with #[juniper::graphql_object] macro, as its pretty close to my previous experiences (such as type-graphql), therefore I would prefer as minimal changes to the current API as possible.
I think the idea of having a JuniperObjectInfo trait to hold the field info for a struct is great. @jmpunkt Thank you for the great idea!
We can even make the changes compatible with the current behaviour if #[juniper::graphql_object] only generates trivial implementations for all fields when explicitly required, such as enable it with #[grapqhl(auto)] attribute.
#[juniper::graphql_object]
#[grapqhl(auto)]
impl User {
pub fn full_name(&self) -> String {
format!("{} {}", self.first_name, self.last_name)
}
}
Gonna work on a P.O.C with my very limited rust knowledge to see if its possible.
Cheers!
Looks like the approach indeed works!! Here is my POC.
One can opt-in this feature by using #[juniper::graphql_object(derive_fields)] attribute on the impl block, and derive([GraphQLObjectInfo]) on the target struct is required.
#[derive(GraphQLObjectInfo, Debug, PartialEq)] // <-- here
#[graphql(scalar = DefaultScalarValue)]
struct Obj {
regular_field: bool,
#[graphql(
name = "renamedField",
description = "descr",
deprecated = "field deprecation"
)]
c: i32,
}
#[juniper::graphql_object(
name = "MyObj",
description = "obj descr",
scalar = DefaultScalarValue,
derive_fields // <-- here
)]
impl Obj {
fn custom_field(&self) -> &str {
"custom field"
}
}
The derived GraphQLTypeInfo looks like this:
impl juniper::GraphQLTypeInfo<DefaultScalarValue> for Obj {
type Context = ();
type TypeInfo = ();
fn fields<'r>(
info: &Self::TypeInfo,
registry: &mut juniper::Registry<'r, DefaultScalarValue>,
) -> Vec<juniper::meta::Field<'r, DefaultScalarValue>>
where
DefaultScalarValue: 'r,
{
<[_]>::into_vec(box [
registry.field_convert::<bool, _, Self::Context>("regularField", info),
registry
.field_convert::<i32, _, Self::Context>("renamedField", info)
.description("descr")
.deprecated(Some("field deprecation")),
])
}
#[allow(unused_variables)]
#[allow(unused_mut)]
fn resolve_field(
self: &Self,
_info: &(),
field: &str,
args: &juniper::Arguments<DefaultScalarValue>,
executor: &juniper::Executor<Self::Context, DefaultScalarValue>,
) -> juniper::ExecutionResult<DefaultScalarValue> {
match field {
"regularField" => {
let res = (|| &self.regular_field)();
juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res
{
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(&(), &r),
None => Ok(juniper::Value::null()),
})
}
"renamedField" => {
let res = (|| &self.c)();
juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res
{
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(&(), &r),
None => Ok(juniper::Value::null()),
})
}
_ => {
{
::std::rt::begin_panic_fmt(&::core::fmt::Arguments::new_v1(
&["Field ", " not found on type "],
&match (
&field,
&<Self as juniper::GraphQLType<DefaultScalarValue>>::name(_info),
) {
(arg0, arg1) => [
::core::fmt::ArgumentV1::new(arg0, ::core::fmt::Display::fmt),
::core::fmt::ArgumentV1::new(arg1, ::core::fmt::Debug::fmt),
],
},
))
};
}
}
}
}
And the generated GraphQLTypeAsync looks like this:
impl juniper::GraphQLTypeAsync<DefaultScalarValue> for Obj
where
DefaultScalarValue: Send + Sync,
Self: Send + Sync,
{
fn resolve_field_async<'b>(
&'b self,
info: &'b Self::TypeInfo,
field: &'b str,
args: &'b juniper::Arguments<DefaultScalarValue>,
executor: &'b juniper::Executor<Self::Context, DefaultScalarValue>,
) -> juniper::BoxFuture<'b, juniper::ExecutionResult<DefaultScalarValue>>
where
DefaultScalarValue: Send + Sync,
{
use futures::future;
use juniper::GraphQLType;
match field {
"customField" => {
let res: &str = (|| "custom field")();
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)
}
_ => {
let v = <Self as juniper::GraphQLTypeInfo<DefaultScalarValue>>::resolve_field(
self, info, field, args, executor,
);
future::FutureExt::boxed(future::ready(v))
}
}
}
}
Feels like it requires a lot of changes, but most of them are really just minor refactoring to be able to reuse the existing functions.
Please let me know hows my implementation and if there is any better approach.
Thanks!
Looks good.
Currently it is not clear what happens on duplicate fields. I would assume that the registry (https://github.com/zhenwenc/juniper/blob/4cdf13396c8c68841d08bb8319cca4055c217980/juniper_codegen/src/util/mod.rs#L888) overrides if the field is already defined. In my opinion silently overriding is not the best approach. We should test upfront if there are possible duplicates and abort (hard error). The user would skip or remove the field in the #[derive(GraphQLObjectInfo)].
Thanks @jmpunkt !
Yes, the derived field resolvers act as fallbacks, where user defined resolvers (in impl block) should take priority. Agree that we should stop the user if there are possible duplicates.
I will wrap up a proper PR with some tests. Cheers!
I thought about the duplicate issue and I think there is no perfect solution. However, I had two ideas in my mind.
- Registry panics if a field is overwritten. Therefore, the program can start but fails immediately. Simple test cases should be able to detect this.
- Use the type system to remember already added such that a generic type
Aholds a list of all added fields. Theaddfunctions of the registry would be constrained such that a function are only callable if there is no typeBinA.Brepresents a single field, thus each fields needs a unique type. So this would probably blow up the compile time and adds complexity to the registry.
Maybe someone gets inspired and finds a better solution.
@jmpunkt Thanks for the advice! Could you please review this PR https://github.com/graphql-rust/juniper/pull/652 when available.
At the moment I perform the duplicate fields check on the GraphQLType#meta method:
https://github.com/graphql-rust/juniper/blob/2e5805fd3892bc2457c4bd34b3adc5cf198f1f3c/juniper_codegen/src/util/mod.rs#L911-L918
which is a runtime check since I couldn't get the struct field information baked in the GraphQLObjectInfo at compile time when expanding the graphql_object macro, but I can be wrong. However, I guess the meta function should only be executed once when creating the graphql schema (RootNode), so the added overhead should be quite minimal 🤞 .
I agree the solution is not perfect though, keen to know if there is any better solution. Cheers!
@jmpunkt @zhenwenc @Victor-Savu
I was thinking about this issue too for quite a while, and here is something to share...
I've thought about the following design:
#[derive(GraphQLObject)] // <-- mandatory
#[graphql(
name = "MyObj",
description = "obj descr",
scalar = DefaultScalarValue,
)]
struct Obj {
regular_field: bool,
#[graphql(
name = "renamedField",
description = "descr",
deprecated = "field deprecation"
)]
c: i32,
#[graphql(ignore)] // or #[graphql(skip)] ?
hidden: bool,
}
#[graphql_object(scalar = DefaultScalarValue)] // <- optional
impl Obj {
fn custom_field(&self) -> &str {
"custom field"
}
fn hidden(&self) -> bool {
self.hidden
}
}
So, the impl Obj {} block is always optional, while the type itself should have #[derive(GraphQLObject)] on it.
As we cannot access from #[derive(GraphQLObject)] proc macro the tokens fed into #[graphql_object] macro, we need some mechanism to register things separately and then reuse them in one place. It seems that inventory crate is widely used exactly for that purpose, so we can: generate separate glue code for resolvers as usual functions outside impl GraphQLType block, and then, in the generated #[derive(GraphQLObject)] code, just inventory::iter() over the all resolvers.
This way, we can have even multiple #[graphql_object] impl Obj { } blocks.
As already has been described above in the discussion, the 2 questions arise here:
- Fields order is undefined. Currently, we generate code in the order it's declared and have no problems. However,
inventory::iter()doesn't provide any order guarantees at all, so we will end up having them randomly swapped every time. The only viable solution I've figured out here, is to useBTreeMapin the code generated forfields()method, so have all the fields alphabetically sorted always. I haven't figured out the way to preserve natural declaration order. - Fields duplication. It's definitely desired to have this check in compile time, while not increasing compilation time a lot. I believe this can be solved by generating a unique sub-module for each field, so if we have duplication,
rustcwill complain about duplication. We can even use those modules naturally, by putting the generated resolver code inside and not polute the current module (something likegql_obj_Obj_hidden::resolve()).
In addition, I think it would be nice to preserve impl Obj {} blocks clean after macro expansion, and reused "as is" in the generated code (at the moment they are just a fiction, not real methods). This will allow for library users to avoid confusions like "where are my rustdocs?".
Thanks for the input. I did not know that the inventory crate exists. To get this clear, we would implement similar functionality (as in inventory) for our registry? Such that we can use the declarative macros for our registry? As a result, the resolver code generate by the #[derive(GraphQLObject)] would only call the collect macro (similar to inventory). Possible problems with the design is compatibility with earlier version since it requires the derive macro for each object. Another problem could be the use of declarative macros in the code generation process (not sure if is possible to generate code which contains declarative macros).
Some notes on your other ideas
- The idea to generate a sub-module ~which includes the fields~ is exactly what I was looking for. It probably does not blowup the compiler time while having decent error messages. We should definitely to this instead of run-time panic.
- Maybe you can open an issue for the "preserve impl Obj {}" feature. Sounds useful but for now I can not imagine how this should look like.
@jmpunkt
To get this clear, we would implement similar functionality (as in inventory) for our registry? Such that we can use the declarative macros for our registry?
No. inventory is a tricky thing: it allows to register things "statically" across crates (AFAIK, registration happens before main() is executed), while collect them in runtime anytime we want (after main() started).
I doubt juniper's registry works this way, nor I see any advantages to piggyback one on another in any way.
What I've meant is that macro expansion contains something like inventory::submit! { ObjResolvers{ /* info about field and resolver fn */ } } for each field and inventory::collect!(ObjResolvers); on #[derive(GraphQLObject)] expansion. And then, when generating code for filling up registry:
fn fields<'r>(
info: &Self::TypeInfo,
registry: &mut juniper::Registry<'r, DefaultScalarValue>,
) -> Vec<juniper::meta::Field<'r, DefaultScalarValue>>
where
DefaultScalarValue: 'r,
{
for field in inventory::iter::<ObjResolvers> {
// fill up registry with info
}
}
Possible problems with the design is compatibility with earlier version since it requires the derive macro for each object.
This way there won't be any compatibility problems, as inventory will be used for each type separately, and if you're not using derive you may not use inventory for your implementation at all.
The only incompatibility is that to codegen implementation you should always have #[derive(GraphQLObject)] on your type. But this is intended by this design to uniform the stuff.
Maybe you can open an issue for the "preserve impl Obj {}" feature. Sounds useful but for now I can not imagine how this should look like.
Hmmm.... I've thought about something like this:
// Preserved code that has been fed into `#[graphql_object]` macro.
impl Obj {
pub fn full_name(&self) -> String {
format!("{} {}", self.first_name, self.last_name)
}
}
// Generated code.
mod gql_Obj_fullName {
use super::Obj;
fn resolve(obj: &Obj, /* unified parameters to call resolver */) -> Result</* unified possible result */> {
// extract required for call here
let res = Obj::full_name(obj);
// wrap into required result
}
}
// Generated code.
inventory::submit! {
__ObjResolver {
name: "fullName",
// and other metadata here...
resolve_fn: gql_Obj_fullName::resolve,
}
}
This way the original code is untouched, while we generate all the required machinery around.
Thanks for the clarifications and examples. Overall I really like the design since it is more "like" Rust. Not sure how well this works at the end.
The only incompatibility is that to codegen implementation you should always have #[derive(GraphQLObject)] on your type. But this is intended by this design to uniform the stuff.
I would like to know what @LegNeato thinks about the whole situation.
A small side note. In terms of unification, there is a similar issue (https://github.com/graphql-rust/juniper/pull/631#issuecomment-620824702). The PR did not unify the interface since it would require a lot of changes in the tests. However, if we are planing to uniform Juniper, then we should enforce this.
@jmpunkt I've read the discussion and I'm a bit more convinced to allow resolvers without &self.
Things like
Therefore, introducing a
&selfmakes it clear that the function requires an underlying object.
are quite vital. And supporting them along seems not to be a big deal.
I always prefer best possible user experience.
@tyranron Thanks for your great ideas!
It's definitely desired to have this check in compile time, while not increasing compilation time a lot. I believe this can be solved by generating a unique sub-module for each field, so if we have duplication, rustc will complain about duplication.
Sounds awesome! I didn't thought of that before.. if I understand correctly, we can generate a dummy struct for each field (eg: __juniper_Obj_fullName), similar to the markers. Is that the right way to do?
Beside, I actually prefer enforcing &self for field resolver functions, not only increase the consistency but also make it more obvious that it is a field resolver for a GraphQLObject instead of an entry point (resolvers for Query / Mutation / Subscription) to the graph.
@zhenwenc
if I understand correctly, we can generate a dummy struct for each field (eg: __juniper_Obj_fullName), similar to the markers
This can be a struct too, but I see a bit more use in generating modules. See the second example above to understand why.
Beside, I actually prefer enforcing &self for field resolver functions, not only increase the consistency
But what if field doesn't really requires &self? What if it's something like associated constant or totally context-dependent thing (like query depth quota for debuging stuff)? In this case &self shows relevance which is really not there. Also, &self may disallow at all any possible constant evaluation for fields.
but also make it more obvious that it is a
field resolverfor aGraphQLObject.
In my practice having #[graphql_object] on top of impl block is quite enought.
But, if we want more distinction, I think there is a reason to consider the following variant, and drop #[graphql_object] macro altogether:
#[derive(GraphQLObject)]
#[graphql(
name = "MyObj",
description = "obj descr",
scalar = DefaultScalarValue,
)]
struct Obj {
regular_field: bool,
#[graphql(
name = "renamedField",
description = "descr",
deprecated = "field deprecation"
)]
c: i32,
#[graphql(ignore)]
hidden: bool,
}
impl Obj {
#[graphql_field(scalar = DefaultScalarValue)]
const fn const_field() -> &str {
"const field"
}
#[graphql_field(scalar = DefaultScalarValue)]
fn hidden(&self) -> bool {
self.hidden
}
fn not_a_field(&self) {
panic!("I'm not a field!")
}
}