Graphql-ruby: connection_type and nulls

[sporto]

When I use connection_type it creates edges and node as nullable fields.

          {
            "name": "edges",
            "description": "A list of edges.",
            "args": [

            ],
            "type": {
              "kind": "LIST",
              "name": null,
              "ofType": {
                "kind": "OBJECT",
                "name": "AwsAccountOrOnboardingEdge",
                "ofType": null
              }
            },
            ...
          },

Does this make sense? I don't see why these should be null. Maybe the edges does, not sure.
At least shouldn't node be NOT_NULLABLE?

[rmosolgo]

It looks like edges is nullable according to the spec: https://facebook.github.io/relay/graphql/connections.htm#sec-Edges

But node may be a non-null type: https://facebook.github.io/relay/graphql/connections.htm#sec-Node

I'm open to supporting non-null node fields, but I don't think we should change the default behavior.

[eapache]

FWIW the vast majority of our connections use non-null nodes. In fact any field we create that is a list defaults to the inner wrapped element being non-null unless otherwise specified.

[sporto]

For a bit more context. We are using Elm for our front end. We started using a library that generates types from our graphql schema. So at the moment for a connection we get something like:

List (Maybe { node = Maybe Thing} )
      ^ edge         ^ node

Having to deal with these nullables in the FE is harder than it should be. We would like to be able to specify a connect_type where nothing is null. So we can end up with:

List { node = Thing }

Thanks

[petrbela]

This is not an issue just with Elm but also TypeScript/Flow or any other type system.

There's a similar discussion for graphql-relay-js. Specifically, it reiterates that edges are always nullable while nodes can be non-null, and that nullability is required in case of an error.

I originally came here to +1 for making the edges non-null but now I'm not so sure. As for nodes, I think those should still be non-nullable.

[petrbela]

For those looking to make everything non-null in their app:

First, define a BaseConnection that makes edges and nodes (if you use them) non-null.

class Types::BaseConnection < GraphQL::Types::Relay::BaseConnection
  def self.edge_type(edge_type_class, edge_class: GraphQL::Relay::Edge, node_type: edge_type_class.node_type, nodes_field: true)
    # Set this connection's graphql name
    node_type_name = node_type.graphql_name

    @node_type = node_type
    @edge_type = edge_type_class

    field :edges, [edge_type_class],
      null: false,
      description: "A list of edges.",
      method: :edge_nodes,
      edge_class: edge_class

    field :nodes, [node_type],
      null: false,
      description: "A list of nodes." if nodes_field

    description("The connection type for #{node_type_name}.")
  end
end

Then define a BaseEdge that makes each node non-null.

class Types::BaseEdge < GraphQL::Types::Relay::BaseEdge
  class << self
    # Get or set the Object type that this edge wraps.
    #
    # @param node_type [Class] A `Schema::Object` subclass
    def node_type(node_type = nil, null: false)
      if node_type
        @node_type = node_type
        field :node, node_type, null: null, description: "The item at the end of the edge."
      end
      @node_type
    end
  end
end

And finally hook them up in your BaseObject:

class Types::BaseObject < GraphQL::Types::Relay::BaseObject
  connection_type_class Types::BaseConnection
  edge_type_class Types::BaseEdge
end

---

Note: Since 1.9.5, GraphQL::Types::Relay::BaseEdge accepts the null param in the node_type method and defaults it to true, which means you can make your node null in each custom edge without having to provide a Types::BaseEdge of your own.

class Types::MyEdge < GraphQL::Types::Relay::BaseEdge
  node_type(MyType, null: false)
end

Still, to default to false, implement your Types::BaseEdge as shown above.

[audiolion]

While I think @petrbela 's solution works, I would rather have official support for this before using it. That being said, there probably needs to be liberal warnings about the tradeoffs that you are making when using non-null types [1], namely:

• Impacts ability to evolve schema (removing fields is now a breaking change as clients expect it to be non-null)
• Errors propagate higher, e.g. if a node is missing the whole edge fails

[1] - https://medium.com/@calebmer/when-to-use-graphql-non-null-fields-4059337f6fc8

[rmosolgo]

I don't have plans to add more support in this area. The built-in classes provide inspiration for your own connection types, which aren't too hard to build, and you could even extend those built-ins. Please let me know if you try it and run into trouble!