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

Enable passing values configuration to GraphQLEnumType as a thunk #4018

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benjie
Copy link
Member

@benjie benjie commented Feb 8, 2024

Many of the type configuration objects in GraphQL-js have properties that accept thunks:

  • GraphQLObjectTypeConfig.fields
  • GraphQLObjectTypeConfig.interfaces
  • GraphQLInterfaceTypeConfig.fields
  • GraphQLInterfaceTypeConfig.interfaces
  • GraphQLInputObjectTypeConfig.fields
  • GraphQLUnionTypeConfig.types

These are because their definitions may contain references to other types which may form cycles, and thus a thunk is required to enable their definition.

This PR allows setting GraphQLEnumTypeConfig.values to be a thunk. At first, this might seem unnecessary (and, well, it has been this way for 9 years so you can be forgiven for thinking that) but it can be quite useful when building enums that reference types. For example: you might have a field that returns a union (Person.pets), and you might want to add an argument that allows you to limit the returned types to only certain types (Person.pets(only:)):

type Person {
  name: String
  pets(only: [AnimalType!]): [Animal]
}
enum AnimalType {
  Cat
  Dog
  Hamster
  Mouse
}
union AnimalType = Cat | Dog | Hamster | Mouse
type Cat { name: String, owner: Person, numberOfLives: Int }
type Dog implements Animal { name: String, owner: Person, wagsTail: Booean }
type Hamster implements Animal { ... }
type Mouse implements Animal { ... }
type Query {
  currentPerson: Person
}
query {
  currentPerson {
    pets(only: [Cat, Dog]) {
      ... on Cat {
        name
        numberOfLives
      }
      ... on Dog {
        name
        wagsTail
      }
    }
  }
}

For this it can be useful for the AnimalType type to enumerate the types in the Animal union, but those types contain references back to AnimalType -> there's a cycle.

The thunk approach outlined in this PR would solve the cycle. There are workarounds (i.e. manually listing out the types) but they're not as ergonomic as (and are more error-prone than) resolving the values directly from the union.

Copy link

netlify bot commented Feb 8, 2024

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 4aef67e
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/65c4eef45b8a0c0008814e72
😎 Deploy Preview https://deploy-preview-4018--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Feb 8, 2024

Hi @benjie, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

Copy link

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great find (and improvement)! All for consistency on these properties either way

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

Successfully merging this pull request may close these issues.

None yet

2 participants