Automatic getters for #[graphql_object]

[Victor-Savu]

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 😿

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! 💯

[LegNeato]

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.

[Victor-Savu]

@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?

[LegNeato]

Yep!

[Victor-Savu]

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.

[Victor-Savu]

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:

• GraphQLData derrived from the struct/enum by #[derive(GraphQLObject)]; and
• GraphQLImpl the implementation of which is generated from the impl block 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 {
...
}

Here is a mock implementation in the rust playground.

[Victor-Savu]

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.

[jmpunkt]

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.

[Victor-Savu]

@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 {
        ...
    }
}

[jmpunkt]

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.

[Victor-Savu]

@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?

[jmpunkt]

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.

[zhenwenc]

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...

[jmpunkt]

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.

[zhenwenc]

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).

[jmpunkt]

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.

[jmpunkt]

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.

[Victor-Savu]

@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!

[zhenwenc]

@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!

[zhenwenc]

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!

[jmpunkt]

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)].

[zhenwenc]

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!

[jmpunkt]

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 A holds a list of all added fields. The add functions of the registry would be constrained such that a function are only callable if there is no typeB in A. B represents 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.

[zhenwenc]

@jmpunkt Thanks for the advice! Could you please review this PR #652 when available.

At the moment I perform the duplicate fields check on the GraphQLType#meta method:

juniper/juniper_codegen/src/util/mod.rs

Lines 911 to 918 in 2e5805f

	if !duplicates.is_empty() {
	duplicates.iter().for_each(|field| {
	panic!("Field with the same name `{}` defined in both struct and impl on type {:?}",
	field,
	<Self as #juniper_crate_name::GraphQLType<#scalar>>::name(info)
	);
	})
	}

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!

[tyranron]

@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:

1. 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 use BTreeMap in the code generated for fields() method, so have all the fields alphabetically sorted always. I haven't figured out the way to preserve natural declaration order.
2. 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, rustc will complain about duplication. We can even use those modules naturally, by putting the generated resolver code inside and not polute the current module (something like gql_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?".

[jmpunkt]

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.

[tyranron]

@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.

[jmpunkt]

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 (#631 (comment)). 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.

[tyranron]

@jmpunkt I've read the discussion and I'm a bit more convinced to allow resolvers without &self.

Things like

Therefore, introducing a &self makes 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.

[zhenwenc]

@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.

[tyranron]

@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 resolver for a GraphQLObject.

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!")
    }
}

[jmpunkt]

I noticed that one use-case is not covered by this design. The "old" design was quite handy in situation where the struct is private. Instead of deriving the GraphQLType it was possible to "implement" it, thus we do not have to wrap the type. In situations where types come from external crates, GraphQLType can be implemented without changes in the external crate. The design allows some kind of layered implementations. For example, someone create a database model and provides two API's, one of them is GraphQL. The other API does not know about Juniper, thus does not require the implemented traits.

Not sure how common this design is. Maybe impl blocks should still be able to generate code?

Regarding the optional &self. Based on your example and the fact that functions should be preserved, &self must be able to be optional. Otherwise it would be useless to preserve the methods. Originally I understood your first example differently. I assumed the whole impl block is optional since the derive marco is doing all the work. But your idea is that if there is directive at the top of the impl, then all functions are fields, otherwise only marked functions are fields?!

[tyranron]

@jmpunkt

But your idea is that if there is directive at the top of the impl, then all functions are fields, otherwise only marked functions are fields?!

Yes. I'm even in favor of leaving the second variant only. To have a computable field, we mark a concrete method rather than an impl block.

Hmmm... thinking about it a bit more shows a downside of this way, as we cannot know about Obj type implicitly in this case (macro won't be provided with that kind of info). So, will be made to do this:

impl Obj {
    #[graphql_field(object = Obj, scalar = DefaultScalarValue)]
    const fn const_field() -> &str {
        "const field"
    }
   #[graphql_field(object = Obj, scalar = DefaultScalarValue)]
    fn hidden(&self) -> bool {
        self.hidden
    }
    fn not_a_field(&self) {
        panic!("I'm not a field!")
    }
}

Which is not quite ergonomic.

So, maybe the second variant really should be an optional one.

I noticed that one use-case is not covered by this design. The "old" design was quite handy in situation where the struct is private. Instead of deriving the GraphQLType it was possible to "implement" it, thus we do not have to wrap the type.

As I've written above, the described design has nothing to do with it and is fully orthogonal to such issues as the current design is. It doesn't disallow you manual implementations of GraphQLType, nor disallows generating code for private structs. All it does - just generates GraphQLType trait implementation in some clever way to unite static fields and computable ones. The fields even doesn't have to be pub, so you may hide them if you don't want to expose in other APIs, while have been exposed in GraphQL schema along.

[zhenwenc]

In term of generating dummy struct for each field in object type to check duplicated fields on compile time, I am not quite sure if it works for all cases, for instance:

mod test_1 {

    #[derive(juniper::GraphQLObjectInfo)]
    #[graphql(scalar = juniper::DefaultScalarValue)]
    pub struct Object {
        field: i32,
    }

    /// Generated by `GraphQLObjectInfo` derive macro:
    struct __juniper_Object_field;
}

mod test_2 {

    use super::test_1::Object;

    #[juniper::graphql_object(scalar = juniper::DefaultScalarValue)]
    impl Object {
        fn field(&self) -> i32 {
            10
        }
    }

    /// Generated by `graphql_object` macro:
    struct __juniper_Object_field;
}

The above code will compile without any error, since the duplicated __juniper_Object_field structs are located in different scope.

Beside, I am a bit conservative about using this approach. As a newbie to rust and as a normal user of the Juniper crate (without knowing its implementation details), I might be confused when seeing error message like this:

   |
34 |     struct __juniper_Object_field;
   |     ------------------------------ previous definition of the type `__juniper_Object_field` here
35 |     struct __juniper_Object_field;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `__juniper_Object_field` redefined here
   |

since these structs are generated by the library, instead of pointing me to the root caused of the issue. It would be way more useful if I see hints like this:

Manual declared field resolver "field" for type "Object" had conflicted with auto-derived field resolvers, you may want to skip the field with `#[graphql(skip)]`).

[tyranron]

@zhenwenc

The above code will compile without any error, since the duplicated __juniper_Object_field structs are located in different scope.

It would be way more useful if I see hints like this:

Manual declared field resolver "field" for type "Object" had conflicted with auto-derived field resolvers, you may want to skip the field with `#[graphql(skip)]`).

🤔... unfortunately, Rust's proc macros doesn't give any type level information, they work only with raw syntax tokens. So we're unable to deeply inspect the information about type outside current declaration, and seems to be that we're unable to ensure any uniqueness outside current declaration. So modules really don't work... meh 😞

The only way I see to resolve this, at the moment, is a not nice and bad hack, but it will work well and will allow to acomplish the described above:
We can generate a file during macro expansion, where to register info about fields for each object, and check for duplicates reading this file. It will allow to provide nice error messages, as we control it on macro expansion level. This file will be required only during macro expansion phase, to ensure global uniqueness of generated fields.
Cons of this approach are:

• proc macros with side effects suck;
• we need to generate a unique file for each build, to not allow the info leak from one build to another, which isn't as trivial, 'cause on macro expansion phase we don't know whether we're called first or last.

[jmpunkt]

I would not recommend generating files and hoping that the compiler executes the macros in the right order. Randomly breaking compilation is annoying. It looks like Rust can not do what we want to do (maybe in the future?). There are currently three different options to go from here

• do not implement this feature
• specifying the simple fields which allows to have compile time checks
• use the experimental implementation derived from previous discussions and use run-time checks

[tyranron]

@jmpunkt

and hoping that the compiler executes the macros in the right order.

Nope, the whole idea is not rely on that. Using the file is a way to achieve that for uniqueness checks, but the downsides are:

• broken macro purity (so we won't potentially be able to use WATT or another sandboxing);
• polluting user's target/ dir with some special files (which isn't a big deal, as for me).

However, investigating this a bit further, it seems that we don't need use files at all. A global static in proc macro crate will do the job, at least for a while. Need more investigation on a question...

At the end, we can support compile-time checks behind a experimental feature gate or something like that.

[jmpunkt]

Ah ok you use files for collisions checks only. I read

where to register info about fields for each object

wrong. Thanks for the clarification.

Is there anything special (or limited) regarding the creation of files during the compilation. I am not familiar with the internals during compilation.

Maybe this is a crazy idea, but if it is possible to auto-generate a separate crate then this crate contains our typing information. Instead putting the generate structs/modules in the current path we put them in a separate crate where we have control over the path. Maybe using build scripts to pre-process the source files?

[zhenwenc]

My original intention was introducing a small improvement to the existing #[graphql_object] macro, therefore I tried to apply minimal changes and avoid any breaking changes in the experimental implementation (#652). If breaking changes and/or hacky solutions are required to achieve this improvement, I would vote for option 1 (do not implement this feature).

However, in practice, I believe this kind of improvement is necessary as field resolvers for object types are so commonly used and critical in GraphQL (otherwise it doesn't seem like a graph..), that said derive([GraphQLObject]) is simply not enough.

Personally, I don't mind doing runtime check (option 3) as I don't see any truely downside of this approach, especially comparing to the tradeoff for the other approaches..

[josejachuf]

Hi

We can say that today we cannot have the introspection of the structure, to obtain the fields, along with the possibility of having custom fields?