Khan / genqlient

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

fix: premarshal structs get generated with omitempty tag #267

Closed kevinmichaelchen closed 1 year ago

kevinmichaelchen commented 1 year ago

This PR addresses a bug described in https://github.com/Khan/genqlient/issues/263

Basically, whenever a struct involves a custom genqlient binding, a secondary "premarshal" struct gets generated.

The bug was that this "premarshal" struct was not propagating the omitempty JSON tags, which was resulting in unexpected behavior.

The fix involved a few changed lines in the Go template, and a few changes in the unit tests.

I have:

benjaminjkraft commented 1 year ago

Thanks! I'll take a closer look later but a few quick comments now:

kevinmichaelchen commented 1 year ago

a schema that looks more like the hasura one

Here's what I'm thinking for that:

Click me ```graphql ################## # Hasura.graphql # ################## query GetPokemon($where: getPokemonBoolExp!) { getPokemon(where: $where) { level } } ################## # schema.graphql # ################## type Query { getPokemon(where: getPokemonBoolExp): [Pokemon!]! } input getPokemonBoolExp { _and: [getPokemonBoolExp!] _not: getPokemonBoolExp _or: [getPokemonBoolExp!] level: IntComparisonExp } input IntComparisonExp { _eq: Int _gt: Int _gte: Int _in: [Int!] _isNull: Boolean _lt: Int _lte: Int _neq: Int _nin: [Int!] } ```
kevinmichaelchen commented 1 year ago

OK, I managed to get snapshot generation working with

UPDATE_SNAPSHOTS=1 go test ./...

The challenge I'm now facing — and maybe this is not relevant to the original ticket — is to get omitempty tags on the fields inside IntComparisonExp

benjaminjkraft commented 1 year ago

Great, thanks! I think IntComparisonExp is another limitation, being discussed in #260 (there are a few ways around but none quite convenient for these hasura schemas). Anyway it's good to have the test; I think this weekend I'm going to try to understand what "make genqlient and hasura not fight so much" might mean more generally.