graphene-django icon indicating copy to clipboard operation
graphene-django copied to clipboard

DjangoConnectionField should have non-null edge and node types

Open jarcoal opened this issue 5 years ago • 8 comments

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?

jarcoal avatar Mar 13 '20 06:03 jarcoal

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

jkimbo avatar Mar 13 '20 10:03 jkimbo

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.

matt-dalton avatar Jul 07 '20 13:07 matt-dalton

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

xp4ck avatar Oct 22 '20 17:10 xp4ck

same problem here!

@jarcoal I also think that you are right

rediska1114 avatar Oct 22 '20 17:10 rediska1114

same problem, has anyone found a workaround for this?

everdrone avatar May 20 '22 22:05 everdrone

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: https://github.com/graphql-python/graphene/pull/1504

garobrik avatar May 04 '23 21:05 garobrik

I believe this issue has been fixed upstream by the release of graphene 3.3.0, since https://github.com/graphql-python/graphene/pull/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)

garobrik avatar Jul 26 '23 18:07 garobrik

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!

tomjohnhall avatar Mar 04 '24 17:03 tomjohnhall