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

Node & edges connection types are nullable #185

Closed
Wenzil opened this issue Nov 22, 2017 · 5 comments
Closed

Node & edges connection types are nullable #185

Wenzil opened this issue Nov 22, 2017 · 5 comments

Comments

@Wenzil
Copy link

Wenzil commented Nov 22, 2017

Currently, typings indicate that the node type of a connection must be nullable. It would be nice if it could be made non-nullable:

const MyConnection = connectionDefinitions({
    nodeType: new GraphQLNonNull(MyNodeType)
});

On the other hand, I feel like the edges field of a connection should always be a non-nullable list. E.g. it should have the type [SomeEdgeType]! rather than [SomeEdgeType]. My reasoning is that a connection with no edges should represent this with an empty list, and not a null value.

Similarly, each edge should be non-nullable. E.g. it should have the type SomeEdgeType! rather than SomeEdgeType.

In short, current connection types look like this:

type MyConnection {
  edges: [{
    node: NodeType
  }]
  pageInfo: { ... }!
}

and it could be improved to:

type MyConnection {
  edges: [{
    node: NodeType! // Nullability is parametrizable
  }!]!
  pageInfo: { ... }!
}
@cliedeman
Copy link

I am also interested in this.

btw it can currently be done with some minor copy-pasting with the current version:

  const edgeType = new GraphQLObjectType({
    name: `${name}Edge`,
    description: 'An edge in a connection.',
    fields: () => ({
      node: {
        type: new GraphQLNonNull(nodeType),
        resolve: resolveNode,
        description: 'The item at the end of the edge',
      },
      cursor: {
        type: new GraphQLNonNull(GraphQLString),
        resolve: resolveCursor,
        description: 'A cursor for use in pagination',
      },
    }),
  });

  const { connectionType } = connectionDefinitions({
    name,
    nodeType: new GraphQLNonNull(nodeType),
    connectionFields: {
      edges: {
        type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(edgeType))),
      },
    },
  });

I started with a PR to make this more seamless but ran into typing issues changing the nodeType from GraphQLObjectType to GraphQLType to allow accepting the new GraphQLNonNull(nodeType). Will try again soon

@maxaggedon
Copy link

I would also be interested in this. As I use connectionFromArray to resolve my connections and it always returns an object with an edges property containing a non-nullable array.

@jaydenseric
Copy link

jaydenseric commented May 10, 2018

According to the spec, edges must be nullable:

This field must return a list type that wraps an edge type …
https://facebook.github.io/relay/graphql/connections.htm#sec-Edges

Nodes may be either nullable or non-nullable:

This field must return either a Scalar, Enum, Object, Interface, Union, or a Non‐Null wrapper around one of those types.
https://facebook.github.io/relay/graphql/connections.htm#sec-Node

@jaydenseric
Copy link

Keep in mind that if the node is non-nullable and it has an error resolving the whole edge is invalidated, a point raised here #103 (comment).

@IvanGoncharov
Copy link
Member

Nodes are fixed in #302

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

No branches or pull requests

5 participants