graphene-django
graphene-django copied to clipboard
DjangoConnectionField should have non-null edge and node types
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 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.
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.
Same problem here! IMO at least we should have an option to make edges and nodes to be required.
same problem here!
@jarcoal I also think that you are right
same problem, has anyone found a workaround for this?
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
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)
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!