Add GraphQLContext derive

[LegNeato]

Is your feature request related to a problem? Please describe. Contexts have some boilerplate, you have to implement juniper::Context which is usually an empty marker trait. The error message when you don't isn't obvious, see https://github.com/graphql-rust/juniper/issues/327.

Describe the solution you'd like It might be good to have a #[derive(GraphQLContext) that does the boilerplate for you. It won't help the error message, but perhaps when people are copying and pasting or reading docs they will be less likely to miss it.

[theduke]

Actually, I think we could just deprecate the Context trait completely and remove the trait bound.

There is no real reason for it to be there.

[theduke]

I guess the only reason for it's existence would be that you can't use a type as context in the macros that isn't intended to be used as a context.

But that would produce compile time errors anyway once you actually construct the schema.

[LegNeato]

Do we want to reserve the trait for future extensibility? If we remove the requirement and later need it for implementation specifics, it would be a breaking change.

[davidpdrsn]

@LegNeato what sort of future extensibility are you thinking of? I imagine adding methods to the trait would also be breaking change.

I'm also in favour of deprecating it.

[theduke]

I don't see a reason to ever add methods on context. More state keeping or functionality would be added to the Executor.

There is a consideration with futures support though: we will probably need a Send + Sync + Clone bound on context! With that in mind, we may want to add those constraints onto the Context trait instead of litterling them around the code everywhere.

[davidpdrsn]

I'm not that familiar with futures, but why Clone?

The app I'm working on has a context like this:

pub struct Context {
    db: DbCon,
    // more things...
}

Where DbCon is

pub struct DbCon(pub PooledConnection<ConnectionManager<PgConnection>>);

PooledConnection comes from r2d2 and is not Clone. So I can't easily make this context Clone.

I could instead store the connection pool itself in the context. The pool is Clone because it just wraps and Arc<SharedPool>. That feels like it would create a lot of churn on the pool though. If each db query would have to first get a new connection.

I also see that PgConnection from Diesel is also not Sync 🤔

[theduke]

but why Clone?

That's a necessary result from concurrency/parallelism. We'll need to clone the Context often.

You could still use a single DB connection per query by wrapping it in Arc<Mutex<Connection>>. (plus a convenience get_connection() method on the Context) Note that the diesel connections are not Sync, but are Send.

But that would of course negate any benefit if most of your resolvers need the db.

[davidpdrsn]

Makes sense. Seems to still be early days for async in Diesel.

[petoknm]

I think we could use a trait alias for this in the future for async. Having an empty trait doesn't make much sense to me. I will try to make a PR to remove it.

[theduke]

@petoknm great that you want to help out, I would hold off on this for now though until I publish my async/await branch and we have a clearer picture of what will be needed.

[petoknm]

Cool, I didn't know that work on async support has already started.