Khan / genqlient

a truly type-safe Go GraphQL client
MIT License
1.07k stars 107 forks source link

Add consideration for pointer false directive when optional: pointer configuration option is used #280

Closed spencermurray closed 1 year ago

spencermurray commented 1 year ago

When support for the optional configuration parameter was added in #198 it was done in a way that, if optional: pointer was set, it would not consider the presence of # @genqlient(pointer: false) graphql file declarations. As a consequence, a schema of

# schema.graphql
scalar DateTime

type Query {
  user(id: ID!): User
}

type User {
  avatarUrl: String
  brandID: ID
  brandVisible: Boolean
  createdAt: DateTime
  firstName: String
  id: ID!
  lastName: String
  name: String
  primaryEmail: String
}

and a query of

# operations.graphql
query userGetByID($id: ID!) {
  user(id: $id) {
    id
   # @genqlient(pointer: false)
    name
  }
}

would result in a generated user struct where name was of type *string

type UserGetByIDResponse struct {
    User * UserGetByIDUser `json:"user"`
}

type UserGetByIDUser struct {
    Id   string  `json:"id"`
    Name *string `json:"name"` // pointer despite query directive
}

With this change the generated structs instead are

type UserGetByIDResponse struct {
    User * UserGetByIDUser `json:"user"`
}

type UserGetByIDUser struct {
    Id   string  `json:"id"`
    Name string `json:"name"` // # @genqlient(pointer: false) directive now respected
}

I do not know if there's appetite for merging this in. To me this updates the behavior to work as I expected and gives more control to users, however, I know that may not be universally agreed upon. I had this fix already made for my own use so I decided to directly open a PR instead of creating an issue first. I updated the existing OptionalPointer test with two new queries that enforce that the configuration option optional: pointer takes effect in absence of # @genqlient(pointer: false) directive and that, if present, the directive takes precedence.

Happy to discuss or make any changes to the PR as requested.


I have:

spencermurray commented 1 year ago

Perfect -- thank you for the timely review!

benjaminjkraft commented 1 year ago

Sorry for the delay in merging -- this week got quite busy!