Relay implementation proposal

[bellini666]

Hey guys,

I saw that relay support is something planned here and I have created a relay implementation that I would like to know if you would be interested as a contribution.

You might have seen that I posted on discord at django channel about this lib I wrote: https://github.com/blb-ventures/strawberry-django-plus. In it I created a generic relay implementation:

• https://github.com/blb-ventures/strawberry-django-plus/blob/master/strawberry_django_plus/relay.py

I made it really generic and no attached to django at all because, not only I would like to have it be useful to non-django related types, but also because my intention was to either contribute it back to you or create a separated project for it (strawberry-relay maybe?). It is also in a single file for that purpose.

The way I did it, you just implement the Node interface by inheriting at the type you are creating and define at least 2 specific methods, one to resolve a single node given its it, and another one to resolve a list of nodes (optionally given a list of ids or none at all, in which the implementation is supposed to return an iterator of all nodes). There are other resolvers that can be overridden by the implementer, but they have a default implementation already.

With that, one can create a connection with some_connection: Connection[NodeType] = relay.connection(). A resolver can also be passed to that (by argument or by decorating a method) which is supposed to return an iterator of the NodeType, and that will be paginated by the field itself. Any arguments added to that resolver will be included together with the first, last, before and after arguments.

There's is a some_node: NodeType = relay.node() which creates a field that expects a global id and returns the node.

And lastly, there's an input mutation field that basically converts all of the arguments passed from the resolver function to an input Input that is automatically created for the mutation, and them converts them back to arguments when calling the function, so for the user it still defines the resolver the same way as a common mutation, without having to create a type for each one and specify it as the single input argument. The only thing I did not do here was to create the Payload type because I did not know how to do that the best way...

A complete example would be:

fruits = [
    {
        "id": 1,
        "name": "Banana",
        "description": "Lorem ipsum",
    },
    {
        "id": 2,
        "name": "Apple",
        "description": None,
    },
    {
        "id": 3,
        "name": "Orange",
        "description": "Lorem ipsum",
    },
]


@strawberry.type
class Fruit(Node):
    name: str
    description: Optional[str]

    @classmethod
    def resolve_node(cls, node_id: str, *, info: Optional[Info] = None, required: bool = False):
        for fruit in fruits:
            if str(fruit["id"]) == node_id:
                return Fruit(**fruit)

        if required:
            raise ValueError(f"Fruit by id {node_id} not found.")

        return None

    @classmethod
    def resolve_nodes(
        cls, *, info: Optional[Info] = None, node_ids: Optional[Iterable[str]] = None
    ):
        node_ids = node_ids and set(node_ids)

        for fruit in fruits:
            if node_ids is not None and str(fruit["id"]) not in node_ids:
                continue

            yield Fruit(**fruit)


@strawberry.type
class Query:
    fruit: Fruit = relay.node()
    fruits_conn: relay.Connection[Fruit] = relay.connection()

    @relay.connection
    def fruits_conn_with_filter(self, name_startswith: str) -> Iterable[Fruit]:
        for fruit in fruits:
            if fruit["name"].startswith(name_startswith):
                yield Fruit(**fruit)


@strawberry.type
class Mutation:
    @relay.input_mutation
    def create_fruit(self, name: str, description: Optional[str]) -> Fruit:
        fruit_data = {
            "id": max(f["id"] for f in fruits) + 1,
            "name": name,
            "description": description,
        }
        fruits.append(fruit_data)
        return Fruit(**fruit_data)

The generated schema would be:

interface Node {
  id: GlobalID!
}

type Fruit implements Node {
  id: GlobalID!
  name: String!
  description: String
}

type FruitEdge {
  cursor: String!
  node: Fruit
}

type FruitConnection {
  edges: [ShipEdge!]!
  pageInfo: PageInfo!
}

type PageInfo {
  hasNextPage: Boolean!
  hasPreviousPage: Boolean!
  startCursor: String
  endCursor: String
}

type Query {
  fruit(id: GlobalID!): Fruit
  fruits_conn(
    before: String,
    after: String,
    first: Int,
    last: Int
  ): FruitConnection!
  fruits_conn_with_filter(
      before: String,
      after: String,
      first: Int,
      last: Int,
      nameStartswith: String!
  ): FruitConnection!
}

input CreateFruitInput {
  name: String!
  description: String
}

type Mutation {
  createFruit(input: CreateFruitInput!): Fruit
}

ORM integrations can implement those resolve_node and resolve_nodes to automatically provide them. I've done that with my lib for the django ORM. I also did override the resolve_id and resolve_connection to make their implementation async-safe, since django orm is not.

[aryaniyaps]

@bellini666 this is a really nice relay implementation! However I think that explicitly declaring connections is better than automatically providing them!

Also, to name relay mutation inputs as MutationName + Input is never actually stated in the relay spec. It's more of a convention generally.. so IMO we should find a way to let users control the input type name.

Apart from that, the implementation looks really good to me!

[bellini666]

@bellini666 this is a really nice relay implementation! However I think that explicitly declaring connections is better than automatically providing them!

Thank you! :)

What do you mean specifically by "explicitly declaring connections"? Don't know if I fully understood the comment.

Also, to name relay mutation inputs as MutationName + Input is never actually stated in the relay spec. It's more of a convention generally.. so IMO we should find a way to let users control the input type name.

Oh yeah, I went that road for the "v0.1" lets say. Adding an optional name should not be hard though, maye an optional input_name: Optional[str] in the @input_connection, which fallback to the MutationName + Input if not provided?

[aryaniyaps]

Oh yeah, I went that road for the "v0.1" lets say. Adding an optional name should not be hard though, maye an optional input_name: Optional[str] in the @input_connection, which fallback to the MutationName + Input if not provided?

Or what about not transforming the inputs at all? We could leave it as it is so that everything becomes simple to the user. We dont need to create inputs implicitly

@strawberry.input
class CreateUserInput:
    name: str

@relay.mutation
def create_user(input: CreateUserInput) -> bool:
    pass

[bellini666]

@aryaniyaps ohhh, I see what you mean now. The idea of the input_mutation was to provide a shortcut for that, but if that isn't the desire of the user, he can simply use a strawberry.mutation instead. All the input mutation does is convert the arguments from the function to an input type, and convert them back to args to send to the resolver on get_result, other than that it is the same as strawberry.mutation.

The reasoning is for people who uses only relay-style mutations (i.e. the ones where the only argument for the mutation is an input), but still want to take advantage of the simplicity of function args -> graphql args typing, without having to create an input for every mutation. I'm one of those for example =P

But If you guys prefer not to have that for strawberry, I don't have any issues at all with removing it from the future-to-come PR.

[AndrewIngram]

A few questions:

1. In the example schema, why does FruitEdge implement Node?
2. How would I add custom fields to an edge type for a particular connection? (i.e. something in addition to just cursor and node)
3. How would pagination work efficiently in connections? Looking at the code, you're just slicing the returned iterable based on the first/last/before/after args; it seems like you're assuming someone is doing limit/offset as the pagination technique, which is somewhat slow. If i wanted to use keyset pagination (which is the primary way to do efficient pagination), i'd need all those connection arguments to be passed into my "resolver".

The issue I see with most attempts at abstracting connections, is that they miss that most of the heavy lifting has to be done in user-land (unless you're making major assumptions, such as using Django's ORM), there's actually relatively little to do in library code.

[bellini666]

Hey @AndrewIngram , thanks for the feedback!

1. Ops, that was a copy/paste mistake at the example itself =P. Fixed it.

2. The implementation there is a generic one that should suffice for most users. To customize it would be required to implement your own Edge and use it instead. The implementation could add some utilities to make it easier to implement your own Edge without having to implement your own Connection also, it should not be too hard.

3. Yes, it is a generic implementation and tendinging to a limit/offset pagination technique, which is a default way of paginating which most users probably use. If one needs full control over before, after, first, last to do some more customized/advanced implementation, he can use @strawberry.field, receive those arguments and resolve them directly.

Having said that, I disagree that it depends on major assumptions, such as using Django's ORM. You can actually paginate anything as long as the nodes you solve (be it in the type itself or the resolver) implements the following interface:

• __len__ to return total amount of nodes that actually exist without requiring to materialize everything as a list
• __getitem__ to convert the slice into something that will materialize only the sliced nodes

For example, one could easily implement a "paginator result" through a REST api by returning an object whose __len__ calls an url to retrieve only the total number of objects, and __getitem__ converts the slice to a limit/offset parameter to call another url to retrieve only those results. The abstraction here helps removing the burden on the user of having to manually convert after/before cursors, generate the edges and page info by hand and only worry about retrieving the slice that was asked from you.

But, I can see one small change that, if we allow the Node type implementer to define a custom connection resolver instead of the default one, one could easily implement something fancier like a keyset pagination. We could also do something crazy, like using 2d slice (the ones that pandas use) to pass all the info to the nodes iterable (e.g. nodes[after, first-last:first, before]), but in this case how would we set the cursor's value? Or even the has_previous_page/has_next_page (is there even a way of defining those in a keyset pagination?)

[AndrewIngram]

Note that i'm using limit/offset to refer to any pattern where you're fetching a page by its index and page size, regardless of whether that's hitting a REST API or running a database query directly.

The Relay pagination spec is kind of designed around the assumption that you're not using limit/offset, because limit/offset is inefficient, that's why it's focused on the more efficient cursor-based pagination (of which keyset pagination is the most common, because it's the easiest to implement). Keyset pagination works based on filtering out all the rows before the cursor (the cursor contains all the necessary data to do this), which means the resolver needs access to at least the cursor being used.

Whilst it's possible to implement limit/offset within the cursor pagination spec, it's not really the right choice for it. I'd implement a separate (but similar) pattern as discussed here: https://andrewingram.net/posts/demystifying-graphql-connections/

One use case the cursor pagination spec is particularly trying to solve well is infinite scrolling, as commonly seen in feed-based apps. This can't be done well with limit/offset because it results in unstable paging and duplicate rows. Cursor-based pagination doesn't suffer from this problem, which is it's preferred. There are downsides to this approach, the primary one being that you can't do direct page access, e.g. starting from page 3 (you don't know its cursor until you fetch it); which is why I like to leave the door open for different pagination systems.

Or even the has_previous_page/has_next_page (is there even a way of defining those in a keyset pagination?)

If you overfetch by one, you can satisfy hasNextPage/hasPreviousPage for whichever direction you're traversing the data. Though you technically can't answer the opposite, because you filtered out all those rows. Even the presence of a cursor doesn't mean anything, because the row before that cursor could've been deleted.

[bellini666]

Hey @AndrewIngram . I totally get your point and agree with you that limit/offset is not the most efficient cursor-based pagination. My point is that it is still good enough for most cases (for some it is on par) and is probably what most people is going to do anyway.

This is a v1 (or even v0.1) solution that can for sure benefit from a refactoring on the way the pagination works to make it easy to do something different, like keyset. I personally would like to implement that possibility now in my solution. I'm going to think about how to do that in a nice way and am also open to suggestions! :)

[aryaniyaps]

Probably the best thing to do now is to not make any assumptions of how users would implement pagination logic. We should leave all of that to the user, as @AndrewIngram suggested. I think we will be left with little code on the strawberry side, but that's okay!

[aryaniyaps]

Also, I think that the relay.mutation field should add a clientMutationId field for every mutation.

[bellini666]

Probably the best thing to do now is to not make any assumptions of how users with implement pagination logic. We should leave all of that to the user, as @AndrewIngram suggested. I think we will be left with little code on the strawberry side, but that's okay!

I would still argue that limit/offset is the most common kind of implementation, but I'm fine with not making assumptions at all. I'm still thinking in a way to make this more generic and less "assumptious" (does that word exist?)

Also, I think that the relay.mutation field should add a clientMutationId field for every mutation.

I also thought about that at the beginning, and after searching for a while I discovered that it was removed from relay and the spec.

[nrbnlulu]

Thank you @bellini666 for this work!
anyone knows if it will it be merged soon?

[K0Te]

I also thought about that at the beginning, and after searching for a while I discovered that it was removed from relay and the spec.

Cool, looks like it has been removed between versions 7 and 8: https://relay.dev/docs/v7.0.0/graphql-server-specification/ https://relay.dev/docs/v8.0.0/graphql-server-specification/
Also, it seems that they have removed the requirement for a single input argument for mutations between https://relay.dev/docs/v10.1.3/graphql-server-specification/#mutations and https://relay.dev/docs/v11.0.0/guides/graphql-server-specification/

What is the best practice now? I am currently using graphene-django, given job = graphene.Field(JobNode),it auto-generates the following return type for a migration:

query: Query
job: JobNode
clientMutationId: String

query might be useful even if clientMutationId is deprecated - it allows to fetch extra info in the scope of the mutation, saving one request.

[bellini666]

Hey guys,

It's been a while since I opened this PR. Since them the implementation has evolved a bit and some issues that were discussed here were resolved.

For example, one of the main issues was that the base implementation was using a limit/offset approach and it was not easy to override that implementation. It is now possible to define a custom Connection class for the Node implemented type and write your own pagination resolver very easily.

The implementation is still self contained and has not dependencies on django specific parts of the lib.

Would like to revive this discussion to see if it makes sense to try to contribute it here to strawberry or if it should become its own library, like strawberry-relay or something like that.

cc @strawberry-graphql/core

[kaos]

I would very much like to see proper support for relay in strawberry, so +1 to make this happen :)

[patrick91]

@kaos how would you see that working? Would we need to provide something like connection_from_array from https://github.com/graphql-python/graphql-relay-py?

I want to make sure we have a relay implementation that's flexible enough to work with many tool, but I'm not sure exactly what that would look like 😊

[kaos]

I've not looked into the particular wrgt strawberry yet. I've used graphene in the past and that worked out well enough, if that helps..?

Given that relay works with generic types (the global Node type) and the like, I imagine that having some basic building blocks for Node types Connections and PageInfo would be enough. I can play with the implementation presented by @bellini666 to see how that works out. It looks pretty much like what I'm hoping for at a quick glance :)

From a user perspective, paging is a real easy endeavour--you get a request for a range of objects that's it. The rest is added complexity to make it efficient and adaptable to many different uses of paginating data which I would be happy to not have to implement/care too much about ;)

[bellini666]

@kaos how would you see that working? Would we need to provide something like connection_from_array from https://github.com/graphql-python/graphql-relay-py?

I want to make sure we have a relay implementation that's flexible enough to work with many tool, but I'm not sure exactly what that would look like blush

My current implementation does create a connection from any iterable and paginates it using limit/offset. But one can basically subclass that Connection and override the from_nodes method for custom pagination behaviour. Like this:

@strawberry.type
class MyConnection(Connection):
    @classmethod
    def from_nodes(
        cls,
        nodes: Iterable[Any],
        *,
        total_count: Optional[int] = None,
        before: Optional[str] = None,
        after: Optional[str] = None,
        first: Optional[int] = None,
        last: Optional[int] = None,
    ):
        ... # custom implementation


@strawberry.type
class MyType(Node):
    CONNECTION_CLASS = MyConnection

Obviously this can be improved when merging into strawberry by either receiving the connection_class as an argumento to strawberry.type or even as a metaclass argument like class MyType(Node, connection_class=MyConnection) (probably better like this, as Node will be have access to it at class creation time)

Given that relay works with generic types (the global Node type) and the like, I imagine that having some basic building blocks for Node types Connections and PageInfo would be enough. I can play with the implementation presented by @bellini666 to see how that works out. It looks pretty much like what I'm hoping for at a quick glance :)

I kept it in a single file and without any django dependency specifically to make it easier to contribute it back here later, so it should (hopefully) be easy to understand what's going on (the django specific code for handling querysets and retrieving node objects are injected when creating the django type).

The module has the basic building blocks with some basic funcionality built-in (like the limit/offset pagination, but with the possibility of easily overriding it) and some fields (to be used as @relay.field or foo: SomeNode = relay.field(...)), where one of those that is a mutation field that automatically creates an input type from a function in a mutation, like:

@strawberry.type
class Mutation:
    @strawberry.field
    def create_person(self, name: str, age: int) -> Person:
        ...

# generates:
"""
type Mutation {
  createPerson(
    name: String!
    age: Int!
  ): Person
}
"""

Would get translated to:

@strawberry.type
class Mutation:
    @relay.input_mutation
    def create_person(self, name: str, age: int) -> Person:
        ...

# generates:
"""
input CreatePersonInput {
  name: String!
  age: Int!
}

type Mutation {
  createPerson(input: CreatePersonInput!): Person
}
"""

[bellini666]

Hey guys,

After my last message and curiously, after coincidently having to define a custom Connection in a personal project, I noticed some things that could be improved in the implementation. I just released a new version there with those improvements: https://github.com/blb-ventures/strawberry-django-plus/releases/tag/v2.0

Those changes make it easier to define custom connections (and thus, add fields to it or define custom pagination methods), and to use them it is just a matter of typing the field with it (no need to define CONNECTION_CLASS anymore)

I also wrote some tests for the relay module, which also contains an example of a custom pagination. It can give you a nice idea on how to use it: https://github.com/blb-ventures/strawberry-django-plus/blob/main/tests/test_relay.py

For curiosity, the schema defined in that test module prints like this: https://github.com/blb-ventures/strawberry-django-plus/blob/main/tests/data/relay_schema.gql

I don't know if there's anything else that needs to be improved in the implementation. There are some code that can be simplified if the integration is merged here, like:

1. The need to define the field like foo: relay.Connection[Foo] = relay.connection() instead of just foo: relay.Connection[Foo] to make sure the correct field is used. Currently there's no way to do that from my lib without monkey-patching strawberry

2. A possibility to allow the type to define an id but still have the Node.id resolver be called to resolve it, which translates it to the base64 notation. You will notice that I defined the Fruit's id as _id in the tests for that reason.

3. Maybe use ID instead of GlobalID for the scalar? I created that to avoid conflicts with the ID provided by strawberry itself, but some people already reported some issues with some clients that really expect the scalar to be called just ID.

cc @patrick91 @kaos

[kaos]

@bellini666 thanks for the update. Hoping to have some feedback for this in the coming week after trying it out in a project I'm working on.

[kaos]

This works beautifully, and mostly does what I would expect.
I only had minor issue with creating a connection field that takes an optional query arg.
Seems it is supposed to work, like this:

        >>> @strawberry.type
        >>> class X:
        ...     some_node: relay.Connection[SomeType] = relay.connection(description="ABC")
        ...
        ...     @relay.connection(description="ABC")
        ...     def get_some_nodes(self, age: int) -> Iterable[SomeType]:
        ...         ...

        Will produce a query like this:

        ```
        query {
            someNode (before: String, after: String, first: String, after: String, age: Int) {
                totalCount
                pageInfo {
...

However, I tried with this:

    rules: relay.Connection[RuleInfo] = relay.connection()

    @relay.connection()
    def get_rules(self, info: Info, query: Optional[RulesQuery] = None) -> List[RuleInfo]:
        request_state = GraphQLContext.request_state_from_info(info)
        return list(

But my schema gives me two fields, the first one is a valid Connection, but the second skips the connection part and is just a list directly:

rules(
  before: String = null
  after: String = null
  first: Int = null
  last: Int = null
): RuleInfoConnection!

getRules(
  before: String = null
  after: String = null
  first: Int = null
  last: Int = null
  query: RulesQuery = null
): [RuleInfo!]!

Not sure if it's a bug or my mistake, but it's close, I can feel it! :)

The diff overall to turn my regular query over to a relay paged connection was very easy. Kudos!

My vote is to get started on a PR to get this relay implementation properly tested and integrated with strawberry 💯

[bellini666]

Hey @kaos ,

Thank you so much for testing it! :)

But my schema gives me two fields, the first one is a valid Connection, but the second skips the connection part and is just a list directly:

That's so strange. I tried to reproduce the issue in my tests to try to fix it and wrote two custom resolvers: https://github.com/blb-ventures/strawberry-django-plus/blob/main/tests/test_relay.py#L122 . One that returns an Iterable[Fruit] and one that returns a List[Fruit]. Both generated the correct schema: https://github.com/blb-ventures/strawberry-django-plus/blob/main/tests/data/relay_schema.gql#L102 . Is there something that you are doing differently from the example in my tests?

The only issue that I found when i wrote tests for it was that the one returning a generator has problems with the pagination slice, so I made a fix for it to use itertools.islice in case nodes doesn't have a __getitem__ attribute. It is available in the 2.0.2 release I just made.

My vote is to get started on a PR to get this relay implementation properly tested and integrated with strawberry 100

😊

I think I'll start a PR soon, so any remaining issues to be resolved can be suggested in the PR itself. Still open to suggestions/issue reports in the meantime

[kaos]

I tried to reproduce the issue in my tests to try to fix it [...]

Hmm.. interesting. I copied your example verbatim with the Fruit type and everythin, and still get a Fruit list rather than a FruitConnection:

fruitsCustomResolverReturningList(
  before: String = null
  after: String = null
  first: Int = null
  last: Int = null
  nameEndswith: String = null
): [Fruit!]!

Your schema looks correct though, so I'm guessing there may be some issue in the relay.py during the move from your repo (I did some minor tweaks to get rid of dependencies to async io stuff and django config settings).

Looking forward to the PR! :)

[bellini666]

I did some minor tweaks to get rid of dependencies to async io stuff and django config settings

It might be it!

Well, when I open the PR I'll port the current tests, and write some new ones, so I should be able to fix any corner case you are hitting there :)

I think I'll have some time next weekend and try to do that!

[bellini666]

Hey guys. Just created a PR with the initial implementation in here: #2511 :)

[rossmeredith]

Note that i'm using limit/offset to refer to any pattern where you're fetching a page by its index and page size, regardless of whether that's hitting a REST API or running a database query directly.

The Relay pagination spec is kind of designed around the assumption that you're not using limit/offset, because limit/offset is inefficient, that's why it's focused on the more efficient cursor-based pagination (of which keyset pagination is the most common, because it's the easiest to implement). Keyset pagination works based on filtering out all the rows before the cursor (the cursor contains all the necessary data to do this), which means the resolver needs access to at least the cursor being used.

Whilst it's possible to implement limit/offset within the cursor pagination spec, it's not really the right choice for it. I'd implement a separate (but similar) pattern as discussed here: https://andrewingram.net/posts/demystifying-graphql-connections/

One use case the cursor pagination spec is particularly trying to solve well is infinite scrolling, as commonly seen in feed-based apps. This can't be done well with limit/offset because it results in unstable paging and duplicate rows. Cursor-based pagination doesn't suffer from this problem, which is it's preferred. There are downsides to this approach, the primary one being that you can't do direct page access, e.g. starting from page 3 (you don't know its cursor until you fetch it); which is why I like to leave the door open for different pagination systems.

Or even the has_previous_page/has_next_page (is there even a way of defining those in a keyset pagination?)

If you overfetch by one, you can satisfy hasNextPage/hasPreviousPage for whichever direction you're traversing the data. Though you technically can't answer the opposite, because you filtered out all those rows. Even the presence of a cursor doesn't mean anything, because the row before that cursor could've been deleted.

Hi. I benefitted from a gist you created years ago where you implemented proper cursor based pagination for the django graphene library. Do you have something similar for strawberry? (I started to use the relay support it offers assuming it would be using proper cursors but it seems it's just masking the index position of the item in the queryset like django graphene).