Skip to content

DjangoConnectionField should have non-null edge and node types #901

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

Open
jarcoal opened this issue Mar 13, 2020 · 8 comments
Open

DjangoConnectionField should have non-null edge and node types #901

jarcoal opened this issue Mar 13, 2020 · 8 comments

Comments

@jarcoal
Copy link

jarcoal commented Mar 13, 2020

There's been a bit of discussion around this in #566 and #560 , but I really think this should get another look.

Currently DjangoConnectionField's edges property is required, but the types inside are not:

edges: [EdgeType]!

Given how DjangoConnectionField works, it's not really possible for EdgeType to be optional. It should look like this:

edges: [EdgeType!]!

And then if you look at the EdgeType itself, node is optional:

node: NodeType

But that also is never going to happen. It should be required:

node: NodeType!

Before I start digging and open a PR, are these assumptions correct? Or am I missing something?

@jkimbo
Copy link
Member

jkimbo commented Mar 13, 2020

@jarcoal I think you're right in your assumptions however the underlying ConnectionField class is defined in Graphene (https://github.com/graphql-python/graphene/blob/master/graphene/relay/connection.py) and the Relay GraphQL spec doesn't define the edges to be required: https://relay.dev/graphql/connections.htm

I think it makes sense for the DjangoConnectionField to have required edges (because of it's implementation) but it might be a bit tricky to implement. If you can create a PR for it that would be great though! Let me know if you need any help.

@matt-dalton
Copy link

I'm also having problems here. With a DjangoConnectionField it is at least possible to work round this by creating your own NonNullConnection class, but I can't find such a solution for DjangoFilterConnectionField.

@xp4ck
Copy link

xp4ck commented Oct 22, 2020

Same problem here! IMO at least we should have an option to make edges and nodes to be required.

@rediska1114
Copy link

same problem here!

@jarcoal I also think that you are right

@everdrone
Copy link

same problem, has anyone found a workaround for this?

@shrouxm
Copy link

shrouxm commented May 4, 2023

I am able to work around this with the following connection subclass:

class MyConnection(Connection):
    @classmethod
    def __init_subclass_with_meta__(cls, node=None, **options):
        type_name = re.sub("Connection$", "", cls.__name__)

        node_for_edge = node
        if node != None and not isinstance(node, NonNull):
            node_for_edge = NonNull(node)

        class Edge(ObjectType):
            node = Field(node_for_edge, description="The item at the end of the edge")
            cursor = String(required=True, description="A cursor for use in pagination")

        class Meta:
            description = f"A Relay edge containing a `{type_name}` and its cursor."

        edge_type = type(f"{type_name}Edge", (Edge,), {"Meta": Meta})

        cls.Edge = edge_type

        cls.edges = Field(
            NonNull(List(NonNull(edge_type))), 
            description="Contains the nodes in this connection.",
        )

        super(MyConnection, cls).__init_subclass_with_meta__(node=node, **options)

This can be used in a subclass of DjangoObjectType like so:

class MyObject(DjangoObjectType):
    class Meta:
        connection_class = MyConnection

As well, I opened this PR which if it lands would obviate the need for a custom subclass: graphql-python/graphene#1504

@shrouxm
Copy link

shrouxm commented Jul 26, 2023

I believe this issue has been fixed upstream by the release of graphene 3.3.0, since graphql-python/graphene#1504 has been merged. It is not the default behavior for backwards compat, but can be made so with the following much simpler connection subclass:

class MyConnection(Connection):
    @classmethod
    def __init_subclass_with_meta__(cls, **options):
        options['strict_types'] = options.pop('strict_types', True)
        super(MyConnection, cls).__init_subclass_with_meta__(**options)

@tomjohnhall
Copy link

I've tried the subclass snippet here and get this error:

...
graphene/relay/connection.py", line 97, in __init_subclass_with_meta__
    assert node, f"You have to provide a node in {cls.__name__}.Meta"
    AssertionError: You have to provide a node in NonNullConnection.Meta

Any examples for implementing that strict_types option with DjangoFilterConnectionField?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants