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

Return null if invalid enum is encountered #2865

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blmhemu
Copy link

@blmhemu blmhemu commented Dec 18, 2023

Describe your PR and link to any relevant issues.

In the current scenario, invalid enums are propagated to the client. This PR sends a null instead - while the graphql spec insists on returning a type error, I found this to be less breaking.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@blmhemu blmhemu marked this pull request as draft December 18, 2023 09:12
@blmhemu
Copy link
Author

blmhemu commented Dec 18, 2023

Hi @StevenACoffman (Sorry if I tagged the wrong person) and gqlgen team,
Wanted to gather information / comments on this topic, before I do full on implementation.

@StevenACoffman
Copy link
Collaborator

Hey, so breaking compatibility with the GraphQL spec is not something most people will want, so I would want to only enable this behavior with a config option and have it default to off.

That said, I'm not sure it is even a common enough desire to add this PR.

@coveralls
Copy link

Coverage Status

coverage: 79.4% (+0.2%) from 79.153%
when pulling 27ca2fc on blmhemu:fix-invalid-enum
into 7dd971c on 99designs:master.

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.

3 participants