Skip to content

Conversation

@lelandbatey
Copy link

This change attempts to address two problems with unmarshaling the results of using GraphQL fragments into two different possible union types. This will hereafter be referred to as "unmarshaling into a union" to keep things short. Those two problems where:

  1. If unmarshaling into a union, and the containing Go struct has fields which are pointers to the unioned structs, then the prior implementation would return an error stating struct field for "<fieldname" doesn't exist in any of 3 places to unmarshal
  2. If unmarshaling into a union, all shared fields of the child union types would be populated in ALL the child fields, not just the child field which had a name maching __typename. This led to lots of duplicated data.

To accomplish this, I've tried to enrich the information being tracked in the decoder "stacks" so that we can track the name of the field that we're marshaling into and the __typename field value in the containing struct.

I've added tests for both of the problems described above.

Based on discussion from here:
#175

This change attempts to address two problems with unmarshaling the
results of using GraphQL fragments into two different possible union
types. This will hereafter be referred to as "unmarshaling into a
union" to keep things short. Those two problems where:

1. If unmarshaling into a union, and the containing Go struct has fields
   which are pointers to the unioned structs, then the prior
   implementation would return an error stating `struct field for
   "<fieldname" doesn't exist in any of 3 places to unmarshal`
2. If unmarshaling into a union, all shared fields of the child union
   types would be populated in ALL the child fields, not just the child
   field which had a name maching `__typename`. This led to lots of
   duplicated data.

To accomplish this, I've tried to enrich the information being tracked
in the decoder "stacks" so that we can track the name of the field that
we're marshaling into and the `__typename` field value in the containing
struct.

I've added tests for both of the problems described above.

Based on discussion from here:
hasura#175
Copy link

@hgiasac hgiasac left a comment

Choose a reason for hiding this comment

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

LGTM. We may add more test cases, but it's good for now. Added a minor comment.

@hgiasac hgiasac merged commit 5d2e076 into hasura:master Nov 4, 2025
1 check passed
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.

2 participants