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

[GraphQL October 2021] Tracking issue #1000

Closed
6 tasks done
ilslv opened this issue Dec 8, 2021 · 10 comments
Closed
6 tasks done

[GraphQL October 2021] Tracking issue #1000

ilslv opened this issue Dec 8, 2021 · 10 comments
Assignees
Labels
enhancement Improvement of existing features or bugfix semver::breaking Breaking change in terms of SemVer
Milestone

Comments

@ilslv
Copy link
Member

ilslv commented Dec 8, 2021

GraphQL October 2021 specification

New version of GraphQL spec is released. Full changelog.

Checklist

@ilslv
Copy link
Member Author

ilslv commented Dec 15, 2021

@tyranron from brief discussion yesterday:

Steps before implementing interface inheritance:

  • Remove support for dyn interfaces and leave only enum.
  • Forbid default implementations of trait methods.
  • Instead of impl Trait on Object, use fields of the #[derive(GraphQLObject)] or #[graphql_object] macros
#[graphql_interface(for = [Human, Droid])]
trait Character {
    fn id(&self) -> &str; // no default impls here
}

#[derive(GraphQLObject)]
#[graphql(impl = Character)] // i guess we can reference here Character instead of CharacterValue
struct Human {
    id: String,
    home_planet: String,
}

struct Droid {
    id: String,
    primary_function: String,
}

#[graphql_object(impl = Character)]
impl Droid {
    fn id(&self) -> &str {
        &self.id
    }

    fn primary_function(&self) -> &str {
        &self.primary_function
    }
}

Unresolved questions:

  • Should we provide impl of the Character to the Human and Droid on the rust side?
  • How do we deal with generic interfaces?

@tyranron
Copy link
Member

tyranron commented Dec 15, 2021

@ilslv

  • Should we provide impl of the Character to the Human and Droid on the rust side?

No. what we do really want here, is for interface being able pick up object fields defined in another manner. Consider this:

#[graphql_interface(for = [Human, Droid])]
trait Character {
    fn id(&self) -> &str;  // the field is effectively `id: !String`
}

#[derive(GraphQLObject)]
#[graphql(impl = Character)]
struct Human {
    id: String,  // so this should wotk out-of-the box
    name: String,
    home_planet: String,
}

struct Droid;

#[graphql_object(impl = Character)]
impl Droid {
    // and this should wotk too,
    // because both field match the _GraphQL signature_ `id: !String`
    fn id(ctx: &Context) -> String {
        ctx.whatever.clone()
    }
}

We do not want to bother here with traits and boilerplate for matching the exact Rust signature. These squats are redundant. We do want to match GraphQL signatures and don't bother about obvious boilerplate.

Though we don't restrict user from implementing the inteface, if he wants too for his needs (for juniper needs it shouldn't be required).

How do we deal with generic interfaces?

With the situation described above, they become simply redundant, as give no benefits. We may still allow them for the purpose of someone's exotic needs. That's not vital. The part is that is vital fot us, is being able to grasp GraphQL field signatures of methods to check/match implementors statically with them.

To make this work, we need to invent some type-level machinery, which will create static materialization of object and interface fields, and match them semi-structurally (we have nominative out-of-the-box as required to specify names of objects/intefaces).

const generics will help here a lot, though we still do lack &str support in them. It's time to use the h!("name") macro-hack turning &str literal into u128.

That would be quite challenging, but deserves to be given a shot, as will simplify users codebases drastically.


Also, for deriving common traits on enum, we need to make it generic under-the-hood:

#[derive(Clone, Copy, Debug)]
enum CharacterValueEnum<A, B> {
  Human(A),
  Droid(B),
}
type CharacterValue = CharacterValueEnum<Human, Droid>;

This way derived traits will propagate structurally withoud compilation error if some variant doesn't satisfy it. (What we do here is actually imitating a compile-time reflection 😊)

@ilslv
Copy link
Member Author

ilslv commented Dec 15, 2021

@tyranron

Should we provide impl of the Character to the Human and Droid on the rust side?

I should have specified, that my question was about deriving traits automatically via proc macro machinery

create static materialization of object and interface fields, and match them semi-structurally

I guess that somewhat similar to the assertion on Event enum in arcana, that we've implemented, that guarantees every leaf of the enum tree to have equal type_ids in case their names are equal.

@tyranron
Copy link
Member

tyranron commented Dec 15, 2021

@ilslv

I guess that somewhat similar to the assertion on Event enum in arcana, that we've implemented, that guarantees every leaf of the enum tree to have equal type_ids in case their names are equal.

Yup, exactly. But far more complicated.

I should have specified, that my question was about deriving traits automatically via proc macro machinery

This still has little sense and will raise additional challenges for us. Consider this example:

#[graphql_interface(for = [Human, Droid])]
trait Character {
    fn id(&self) -> &str; 
}

struct Droid;

#[graphql_object(impl = Character)]
impl Droid {
    fn id(ctx: &Context) -> String {
        ctx.whatever.clone()
    }
}

From the Rust prespective we cannot just implement Character for Droid here, because:

  • Character::id() provides no Context to pass into Droid::id();
  • Lifetime check will fail on turning String to &str and returning it.

And we will hit a lot of such situations.

While what we can do is to have in our type-machinery some unified form of calling a GraphQL field (expanded on #[graphql_object]/#[derive(GraphQLObject)] side):

impl Ufcs<Field<"id">> for Droid {
    type Result = String;

    fn call(self, field: Field<"id">, args: FieldArgs, ctx: &Executor) -> (self, Self::Result) {
        (self, Self::id().into())
    }    
}

impl Ufcs<Field<"id">> for Human {
    type Result = String;

    fn call(self, field: Field<"id">, args: FieldArgs, ctx: &Executor) -> (self, Self::Result) {
        (self, self.id.into())
    }    
}

(modulo async separation)

This won't be user-facing and will allow us to deal with signatures in a uniform way.

So in reality, we don't ever need to call the declared trait methods. As in GraphQL schema lnaguage, they will only define the necessary GraphQL signatures for us, and using those signatures we will call the actual object methods/fields.

@ilslv
Copy link
Member Author

ilslv commented Dec 15, 2021

Also on the note of interface inheritance.

Earlier I proposed to reuse a syntax of trait extending other traits

#[graphql_interface]
trait Character {
    fn id(&self) -> ID;
}

#[graphql_interface]
trait Named: Character {
    fn name(&self) -> String;
}

But after reading a great article on trait inheritance, I found a problem: in case field on the interface return another interface we should be able to declare interface, that imposes even more requirements.
Example from the article:

interface Edge {
  cursor: String!
  node: Node
}

type AnimalEdge implements Edge {
  cursor: String!
  node: Animal # implements Node itself
}

translated into Rust

#[graphql_interface(for [AnimalEdge, AfricanElephantEdge, AlalaEdge])]
trait Edge {
    fn cursor(&self) -> &str;
    fn node(&self) -> NodeValue;
}

#[graphql_interface(for [AfricanElephantEdge, AlalaEdge], impl = Edge)]
trait AnimalEdge {
    fn cursor(&self) -> &str;
    fn node(&self) -> AnimalValue;
}
Full example from the article
#[derive(GraphQLObject)]
struct PageInfo {
    has_previous_page: bool,
    has_next_page: bool,
    start_cursor: String,
    end_cursor: String,
}

#[graphql_interface(for [Animal, AfricanElephant, Alala])]
trait Node {
    fn id(&self) -> ID;
}

#[graphql_interface(for [AnimalEdge, AfricanElephantEdge, AlalaEdge])]
trait Edge {
    fn cursor(&self) -> &str;
    fn ndoe(&self) -> NodeValue;
}

#[graphql_interface(for [AnimalConnection, AfricanElephantConnection, AlalaConnection])]
trait Connection {
    page_info: PageInfo,
    edges: Vec<EdgeValue>,
}

#[graphql_interface(for [AfricanElephant, Alala], impl = Node)]
trait Animal {
    fn id(&self) -> ID;
    fn name(&self) -> &str;
    fn mother(&self) -> AnimalValue;
    fn father(&self) -> AnimalValue;
    fn children(&self) -> AnimalConnectionValue;
}

#[graphql_interface(for [AfricanElephantEdge, AlalaEdge], impl = Edge)]
trait AnimalEdge {
    fn cursor(&self) -> &str;
    fn node(&self) -> AnimalValue;
}

#[graphql_interface(for AfricanElephantConnection, impl = Connection)]
trait AnimalConnection {
    fn page_info(&self) -> PageInfo;
    fn edges(&self) -> Vec<AnimalEdgeValue>;
}

#[graphql_object(impl [AnimalEdge, Edge])]
struct AfricanElephantEdge {
    cursor: String,
    node: AfricanElephant,
}

#[graphql_object(impl [AnimalConnection, Connection])]
struct AfricanElephantConnection {
    cursor: String,
    node: AfricanElephant,
}

#[graphql_object(impl = [Animal, Node])]
struct AfricanElephant {
    id: ID,
    name: String,
    mother: Box<AfricanElephant>,
    father: Box<AfricanElephant>,
    children: Box<AfricanElephantConnection>,

    left_tusk_length: f64,
    right_tusk_length: f64,
}

#[graphql_object(impl [AnimalEdge, Edge])]
struct AlalaEdge {
    cursor: String,
    node: Alala,
}

#[graphql_object(impl [AnimalConnection, Connection])]
struct AlalaConnection {
    cursor: String,
    node: Alala,
}

#[graphql_object(impl = [Animal, Node])]
struct Alala {
    id: ID,
    name: String,
    mother: Box<Alala>, 
    father: Box<Alala>,
    children: Box<AlalaConnection>,

    beak_lenght: f64,
}

@tyranron
Copy link
Member

@ilslv agree, as I see, we should declare all the interface fields anyway, despite it implements another one with similar fields. So we should be close here to what GraphQL does, not Rust.

Another good reasong for this is being similar on syntax with objects.

The interesting thing here, is that with this design we may use structs to define GraphQL interfaces, because we treat them only as a fields set, which is quite handy btw:

#[graphql_interface(for = [AnimalEdge, AfricanElephantEdge, AlalaEdge])]
trait Edge {
    fn cursor(&self) -> &str;
    fn node(&self) -> NodeValue;
}

#[derive(GraphQLInterface)]
#[graphql(for = [AfricanElephantEdge, AlalaEdge], impl = Edge)]
struct AnimalEdge {
    cursor: String,
    node: AnimalValue,
}

@ilslv
Copy link
Member Author

ilslv commented Dec 16, 2021

@tyranron I really do like approach with structs, as it clearly shows, that it's for declaration use only and shouldn't be implemented. But upon playing around with it I've found 1 problem: asyncness. With traits we can enforce async-only behaviour with async method, but with structs it looks like we'll have to use attributes:

#[derive(GraphQLInterface)]
#[graphql(for = [AfricanElephantEdge, AlalaEdge], impl = Edge)]
struct AnimalEdge {
    cursor: String,
    #[graphql(async)]
    node: AnimalValue,
}

UPD: One more problem with structs is that you forced to specify owned return value, while actual implementation can return borrowed value

trait Field<S, const N: u128> {
    type Context;
    type Out;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out;
}

struct Human {
    name: String,
}

#[graphql_object(impl Character)]
impl Human {
    fn name(&self) -> &str {
        &self.name
    }
}

#[automatically_derived]
impl<'me, S> Field<S, { hash("name") }> for &'me Human {
    type Context = ();
    type Out = &'me str;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        self.name()
    }
}

#[derive(GraphQLInterface)]
#[graphql(for Human)]
struct Character {
    name: String,
}

#[automatically_derived]
impl<'me, S> Field<S, { hash("name") }> for &'me CharacterValue {
    type Context = ();
    type Out = &'me String;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        match self {
            CharacterValue::Human(v) => {
                <_ as Field<S, { hash("name") }>>::call(v, args, executor).into()
                //                       Failed to convert &str to &String ^^^^^^
            }

        }
    }
}

@ilslv
Copy link
Member Author

ilslv commented Dec 16, 2021

Encountered one more obstacle with abstracting over structs and methods returning be reference and value

trait Field<S, const N: u128> {
    type Context;
    type Out;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out;
}

struct Human<'a> {
    name: String,
    planet: &'a str,
}

impl<'a, 'me, S> Field<S, { hash("name") }> for &'me Human<'a> {
    type Context = ();
    type Out = &'me String;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        &self.name
    }
}

impl<'a, 'me, S> Field<S, { hash("planet") }> for &'me Human<'a> {
    type Context = ();
    type Out = &'me &'a str; // maybe thats ok

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        &self.planet
    }
}

struct Droid {
    name: String,
    planet: String,
}

impl Droid {
    fn name(&self) -> String {
        self.name.clone()
    }

    fn planet(&self) -> &str {
        &self.planet
    }
}

impl<'a, 'me, S> Field<S, { hash("name") }> for &'me Droid {
    type Context = ();
    type Out = String;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        self.name()
    }
}

impl<'a, 'me, S> Field<S, { hash("planet") }> for &'me Droid {
    type Context = ();
    // To achive this we basically need to to the compilers job
    // which is impossible in general case with elided lifetimes.
    type Out = &'me str;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        self.planet()
    }
}

@tyranron
Copy link
Member

@ilslv

But upon playing around with it I've found 1 problem: asyncness. With traits we can enforce async-only behaviour with async method,

As we do treat them as GraphQL field signatures, we ideally shouldn't bother about asyncness here, because GraphQL doesn't have that notion.

We do have clear "rules" about asyncness:

  • sync fields are OK to be called from async context
  • async fields cannot be called from sync context

So, following the machinery described two comments above, for each GraphQL object field we should generate two call imls: UfcsSync<Field<"id">> and UfcsAsync<Field<"id">>. So, async context will call UfcsAsync impls and sync context, accordingly, will call UfcsSync impls. If we would try to call an async field from sync context, we would receive a compilation error due to missing impl.

One more problem with structs is that you forced to specify owned return value, while actual implementation can return borrowed value

Again, the idea is not to specify the exact type that should be used by Rust, but a GraphQL type that should be satisfied by Rust type.

Ideally, it should be OK for a field to return &str where interface field specifies String, as they both refer to the same type !String GraphQL type. We also should be capable of that, because GraphQLValue method doesn't check itself what is returned actually by resolved field, implying all the checks where made before.

impl<'a, 'me, S> Field<S, { hash("planet") }> for &'me Droid {
    type Context = ();
    // To achive this we basically need to to the compilers job
    // which is impossible in general case with elided lifetimes.
    type Out = &'me str;

    fn call(
        self,
        args: &::juniper::Arguments<S>,
        executor: &::juniper::Executor<Self::Context, S>,
    ) -> Self::Out {
        self.planet()
    }
}

Yeah, may be we shouldn't bind the returned type, but rather do its resolution in-place, as we do in GraphQLValue::resolve_field method. Seems that the signature will become almost identical, and what we are really trying to do here is to make GraphQLValue granular over fields.

@tyranron
Copy link
Member

@ilslv as we have gone here into new interfaces design deeply, I'd like to propose move this conversation to the appropriate PR, as this becomes offtopic for this issue.

tyranron pushed a commit that referenced this issue Dec 20, 2021
…ttribute argument in `#[graphql_scalar]` and `#[derive(GraphQLScalarValue)]` macros (#1003, #1000)

- support `isRepeatable` field on directives
- support `__Schema.description`, `__Type.specifiedByURL` and `__Directive.isRepeatable` fields in introspection
@ilslv ilslv mentioned this issue Dec 20, 2021
15 tasks
tyranron pushed a commit that referenced this issue Jan 26, 2022
- remove support for `#[graphql_interface(dyn)]`
- describe all interface trait methods with type's fields or impl block instead of `#[graphql_interface]` attribute on `impl Trait`
- forbid default impls on non-skipped trait methods
- support additional nullable arguments on implementer
- support returning sub-type on implementer
tyranron pushed a commit that referenced this issue Feb 24, 2022
- support generic scalars
- make it applicable to type aliases and struct/enums/unions
@tyranron tyranron added this to the 0.16.0 milestone Apr 13, 2022
tyranron added a commit that referenced this issue Jun 28, 2022
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 semver::breaking Breaking change in terms of SemVer
Projects
None yet
Development

No branches or pull requests

2 participants