cjoudrey / graphql-schema-linter

Validate GraphQL schema definitions against a set of rules
MIT License
694 stars 62 forks source link

Extend connection argument specs to input types #259

Open LunaticMuch opened 4 years ago

LunaticMuch commented 4 years ago

The rule relay-connection-arguments-spec fails when pagination arguments are wrapped in an input type like:

type Query {
  myquery(pagination: pageInput): myConnection
}

input pageInput {
  first: Int
  after: String
}

This construct is technically valid and wrapping arguments in input types is also a best practice. Relay specs do not address specifically the point, either saying it can or cannot be done. When both forward and backward pagination is allowed, an input type with 4 fields delivers a better DX. The rule should loop into all input types and check if an input type is solving for pagination, despite its name.

cjoudrey commented 4 years ago

👋 @LunaticMuch thanks for opening this issue.

The relay-connection-arguments-spec rule was written to adhere to this: https://relay.dev/graphql/connections.htm#sec-Forward-pagination-arguments

While it doesn't explicitly say that the first / after argument aren't wrapped, this is what all the examples on this page and Relay docs show.

Full disclosure, I haven't followed the latest development on this and it's possible that wrapping these arguments in an object is now a common practice.

If you can show me some Relay references of where first / after arguments are wrapped in an input type, I'd happily reconsider this.

LunaticMuch commented 4 years ago

The rule perfectly adheres to the spec. My idea is to extend it in order to make it able to check if pagination arguments are wrapped into input types. Regarding best practices, I can try to source something. I have to be honest saying that most of best practices regarding input types are for mutations, but there's no downside in extending them for queries. Would you prefer this:

type Query {
  myQuery(
    id: String
    status: statusEnum
    first: Int
    last: Int
    before: String
    after: String
    orderFiled: [String]
    direction: dirEnum
    onlyActive: Boolean
    fromDate: Date
    toDate: Date
  ): [Something]
}

or this

type Query {
  myQuery(
    filter: QueryInput
    order: OrderInput
    pagination: PageInput
  ): [Something]
}

input QueryInput {
  id: String
  status: statusEnum
  onlyActive: Boolean
  fromDate: Date
  toDate: Date
}

input OrderInput {
  orderFiled: [String]
  direction: dirEnum
}

input PageInput {
  first: Int
  last: Int
  before: String
  after: String
}

I will try to come back with more info.