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

Allow elements in edges to be marked as NonNull #968

Closed
Jaspaul opened this issue May 17, 2019 · 14 comments
Closed

Allow elements in edges to be marked as NonNull #968

Jaspaul opened this issue May 17, 2019 · 14 comments

Comments

@Jaspaul
Copy link

Jaspaul commented May 17, 2019

It seems that an edge, within the connection edges, cannot be marked as NonNull. I believe this is the result of https://github.com/graphql-python/graphene/blob/master/graphene/relay/connection.py#L103 where the edges field is hard coded to NonNull(List(edge)).

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

This request is motivated by the following use case where we have a collection of objects, which we can guarantee are never null.

See the following for a contrived example:

type PostConnection {
  pageInfo: PageInfo!
  edges: [PostEdge]!
}

Since we know that a PostEdge, if found, will never be null we would like to be able to specify the following:

type PostConnection {
  pageInfo: PageInfo!
  edges: [PostEdge!]!
}
@stale
Copy link

stale bot commented Jul 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 29, 2019
@jkimbo jkimbo removed the wontfix label Jul 30, 2019
@valberg
Copy link

valberg commented Aug 21, 2019

Is anyone doing work on this? Otherwise I might give it a go - it would make using elm-graphql with graphene relay much nicer :)

@ChazZeromus
Copy link

ChazZeromus commented Sep 9, 2019

Agreed, the way graphene handles automatic connection schema creation makes it difficult to specify non-null edges in the list.

At least in graphene's Connection class, it's nice that it considers if there's an Edge class already defined so I'm able to slightly do a bit of overriding to get the element non null. This will work:

class PostConnection(graphene.Connection):
    class Meta:
        node = Post

    class PostEdge(graphene.ObjectType):
        node = graphene.Field(Post, required=True)
        cursor = graphene.String(required=True)

    edges = graphene.List(graphene.NonNull(PostEdge), required=True)

Produces:

type PostConnection {
  """Pagination data for this connection."""
  pageInfo: PageInfo!
  edges: [PostEdge!]!
}

type PostEdge {
  node: Post!
  cursor: String!
}

You just gotta add the descriptions back in :)

Not sure what would be the nicest way to make the edge element non-null if doing such a thing was supported, maybe a meta field on the connection class?

@p7g
Copy link

p7g commented Oct 2, 2019

This can be nicely wrapped up in a subclass of Connection:

class NonNullConnection(graphene.relay.Connection, abstract=True):
    @classmethod
    def __init_subclass_with_meta__(cls, node, **kwargs):
        if not hasattr(cls, 'Edge'):
            _node = node
            class EdgeBase(graphene.ObjectType, name=f'{node._meta.name}Edge'):
                cursor = graphene.String(required=True)
                node = graphene.Field(_node, required=True)

            setattr(cls, 'Edge', EdgeBase)

        if not hasattr(cls, 'edges'):
            setattr(cls, 'edges',
                    graphene.List(graphene.NonNull(cls.Edge), required=True))

        super(NonNullConnection, cls).__init_subclass_with_meta__(
            node=_node,
            **kwargs
        )

Which can be used like this (assuming a type called User):

class UserConnection(NonNullConnection):
    class Meta:
        node = User

# or

UserConnection = type('UserConnection', (NonNullConnection,), {}, node=User)

And results in a schema like this:

type UserConnection {
  pageInfo: PageInfo!
  edges: [UserEdge!]!
}

type UserEdge {
  cursor: String!
  node: User!
}

@stale
Copy link

stale bot commented Dec 31, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 31, 2019
@stale stale bot closed this as completed Jan 14, 2020
@jckw
Copy link

jckw commented Jun 10, 2020

Did this ever get solved in the library? I've been using @p7g's workaround for the last 6 months or so which is working fine - but this seems like a fairly important option now that everyone's using type generation with their GraphQL libraries.

@jkimbo
Copy link
Member

jkimbo commented Jun 11, 2020

@jckw looks like it hasn't been resolved so I'm reopening. If you have time to create a PR that would be very helpful. Ideally specifying values as NonNull should be configurable since there conceivably cases where that is what you want. However they should be NonNull by default.

@jckw
Copy link

jckw commented Jun 27, 2020

Do you mean they should not be NonNull by default? Otherwise it might break backwards compatibility I think

@ghost
Copy link

ghost commented Apr 15, 2021

Anyone working on this? I regularly bump into this requirement.

@Lekesoldat
Copy link

Bump 🙏🏻

@alexaor
Copy link

alexaor commented Dec 9, 2021

bump as well

@pfcodes
Copy link

pfcodes commented Jun 22, 2022

+1

@erikwrede
Copy link
Member

Client controlled nullability will fix this without having to interfere with the relay spec: graphql/graphql-spec#867

@erikwrede
Copy link
Member

Released in 3.3.0. Please see the release notes to learn about the caveats of using this feature and why it might be worthwhile to wait for client controlled nullability instead.

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

10 participants