New rule: force Relay schema structure

[dotansimha]

Related: https://relay.dev/docs/en/graphql-server-specification.html#schema

[ilyavolodin]

This is very tricky to implement, since there isn't a good way to figure out if returned type is supposed to be paginated. This will ether have to be done through naming conventions (inconvenient for the end user), or check return type for arrays and assume that any array should be paginated (probably an overkill).

[dotansimha]

Reopening, just because I think we can implement some aspects of this.

@B2o5T what do you think about the following:

• If interface Node exists in the schema, I think we can enforce that types that contain id: ID! field will have to implement Node interface.
• Require Connection/edges types on fields that return raw GraphQL list types?

[connorjs]

My top reasons

Sharing some thoughts for why I think an ESLint can drive value. Top reasons include

1. Easier for schema authors to adopt the specification (fire up linting and profit)
2. Removes possibly extraneous items from the schema. (I say possibly b/c some clients may have interest in generic Connection or Edge types, but I think those interfaces off limited utility to clients.)

I would love to chat more about this. As some of my later thoughts show, this rests at the forefront of my schema design right now.

---

Other comments

have to be done through naming conventions (inconvenient for the end user)

The GraphQL Cursor Connections Specification requires these naming conventions. So, if we promote the rule as explicitly for this spec, I think the naming convention concern disappears.

check return type for arrays and assume that any array should be paginated (probably an overkill)

Yeah, I do not know how we find the exact pagination fields. Maybe we look for fields that have conforming Arguments. I do not have context about how the ESLint plugin works w.r.t. analyzing the GQL AST.

If interface Node exists in the schema, I think we can enforce that types that contain id: ID! field will have to implement Node interface.

First, I think this stems from the Global Object Identification spec, but +1 to having this. strict-id-in-types may handle this already, too.

---

More context

I set out to implement cursor based pagination with schema-driven interfaces. Roughly,

interface Connection {
  edges: [Edge]
  pageInfo: PageInfo!
}

interface Edge {
  node: Node
  cursor: Cursor
}

scalar Cursor

type PageInfo { ... }

Then I would implement the interface with a specific connection.

extend Query {
  foos(first: NonNegativeInt, after: Cursor): FooConnection
}

type FooConnection implements Connection {
  edges: [FooEdge] # Works fine
  pageInfo: PageInfo!
}

type FooEdge implements Edge {
  node: FooResult # Works with `Foo`, but not `FooResult`
  cursor: Cursor
}

type Foo { ... }

union FooResult = Foo | ...

However, the *Result union approach breaks the schema because Edge expects node to have type Node, but unions cannot implement interfaces. (At least not yet, see graphql/graphql-spec#518.)

So, I had two options: (1) Remove node: Node from the Edge interface or (2) do not use union *Result approach. While I leaned towards (1), I realized that maybe linting could satisfy all of this. Hence my essay here.

[dimaMachina]

@connorjs @domkm I have implemented 3 new rules according to Relay spec for validating Connection types, Edge types and PageInfo object. The rule for Arguments spec will be implemented later.

Also for validation Edge types, I added 3 configurable options that don’t strict in Relay spec:

• Enforce that Edge type end in “Edge”
• The edge type’s field node must implement Node interface
• A list type can only wrap an edge type

Before these rules will be merged I need feedback, can you test it on your schemas? You can try an alpha version @graphql-eslint/eslint-plugin@3.10.0-alpha-4a6927c.0.

Docs:
https://github.com/B2o5T/graphql-eslint/blob/relay-connection-types/docs/rules/relay-connection-types.md
https://github.com/B2o5T/graphql-eslint/blob/relay-connection-types/docs/rules/relay-edge-types.md
https://github.com/B2o5T/graphql-eslint/blob/relay-connection-types/docs/rules/relay-page-info.md

Also, you can extend plugin:@graphql-eslint/relay config that enables all 3 new rules

[connorjs]

TL;DR - Can we (a) add relay to plugin array literal and (b) add support for a Scalar type instead of String for cursors. Otherwise, AWESOME 😎

---

@B2o5T - Do we need to add the plugin (relay) to this array?

I have added the rules directly in the mean-time. 👍🏻 My feedback follows.

1. (page-info) Thoughts on supporting scalars? I use a Cursor scalar in my schema instead of String directly (for semantics + mocking). Unsure how we would do this, maybe with an option for cursor scalar type, which defaults to String? Otherwise, the enforce non-null works great! I believe a scalar serialized as a string satisfies the spec.

2. (edge-types + connection-types) My current interface approach (mentioned above) fails these rules, but I think that represents the correct state.

   1. It complains (correctly) that my Edge type does not have node (b/c I use unions, so I cannot have node).

   2. And that my Connection interface is not an object (also arguably correct).

   In both cases, I would just disable the rule for those specific lines. Furthermore, I will probably remove my interfaces with these ESLint rules 😎, so ZERO concerns (but wanted to share feedback for completeness + documenting for others).

   Edit: In fact, I just removed all of my interfaces in favor of these rules. Works great! 🎉

[dimaMachina]

@connorjs Thank you for your feedback 😍💪

Do we need to add the plugin (relay) to this array?

Exactly, this was my mistake, now this should be fine as I added also a test to test that the relay config is properly loaded.

(page-info) Thoughts on supporting scalars?

I think we can support by default a non-null String/Scalar for startCursor/endCursor as we do for the cursor field in edge types.

(edge-types + connection-types) My current interface approach (mentioned above) fails these rules, but I think that represents the correct state.

For your particular case if you still want to use interface Connection approach you can simply use eslint-disable directive to disable the error report above your interface definition.

You can try @graphql-eslint/eslint-plugin@3.10.0-alpha-493a124.0 alpha version with applied suggestions

UPDATE:

Added 4th rule and last for relay spec relay-arguments https://github.com/B2o5T/graphql-eslint/blob/relay-connection-types/docs/rules/relay-arguments.md
You can try @graphql-eslint/eslint-plugin@3.10.0-alpha-c2f3596.0 alpha version

[dimaMachina]

Available in @graphql-eslint/eslint-plugin@3.10.0