Nulls are now unmarshalled as zero values for primitive types #3162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Steve Coffman <steve@khanacademy.org>
|
Hey, thanks! I updated this PR to make the same changes for unsigned integers. While your PR is clearly in line with the Go standard JSON library, it's not necessarily the expectation of the JavaScript developers making GraphQL calls to gqlgen. As a result, this might be a controversial change. If people find that the GraphQL spec requires different behavior for null values, I would be open to reverting this change. In the absence of that, I think this will make day-to-day life better for developers, so I very much appreciate you contributing it. |
giautm
added a commit
to giautm/gqlgen
that referenced
this pull request
Sep 7, 2024
…99designs#3162)" This reverts commit d0e6892.
giautm
added a commit
to ent/contrib
that referenced
this pull request
Sep 7, 2024
GQLGen changed its behavior in 99designs/gqlgen#3162 to return a zero value for `null`. Which causes `first` / `last` to receive the 0 value, and fail the check for supplying both `first` and `last`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The current implementation of Unmarshal to primitives in gqlgen does not handle null values consistently. Specifically, when input types are mapped to Go structs, optional fields defined as primitives fields in the Go struct can lead to errors or unexpected behaviors.
Example
Consider the following GraphQL input type:
This maps to the following Go struct:
When unmarshalling the following JSON input:
{ "name": null, "age": null }The resulting
Namefield in the Go struct is assigned the string value"null", and theAgefield causes an error. This behavior is inconsistent with the Go standard JSON library, which leaves the field untouched.Proposed Solution
To align with the Go standard library's behavior, null values should be mapped to the zero-value of the corresponding Go type. This approach is more idiomatic to Go and reduces the potential for errors.
I have: