search

99designs / gqlgen

go generate based graphql server library
https://gqlgen.com
MIT License
9.96k stars 1.17k forks source link

Massive undocumented breaking change in v0.17.50 #3321

Open Vilsol opened 3 weeks ago

Vilsol commented 3 weeks ago

I have hit a major issue due to this undocumented breaking change (https://github.com/99designs/gqlgen/pull/3243), which was introduced in a patch version bump.

This is how the resolver.go looked like before on version v0.17.49: https://github.com/satisfactorymodding/smr-api/blob/179d26156ff1bfc48119ab24ee8a4740d2a2a18b/gql/resolver.go

This is how it looks like after running the generator on version v0.17.50: https://gist.github.com/Vilsol/1d7ac5dabd75933604449e1d6faa3323

Is there some config options I can change to make this work with newer versions? Or is my only possible path to not upgrade?

JonasDoe commented 3 weeks ago

Gotta agree, I don't really understand what the intention behind this change was. My situation isn't as dramatic as in Vilsol's example, I just added some fields to the Resolver which I probably have to pass by ctx by now, of with some global variables, or come up with my own RootResolver and drop the generated one.

But what's the gain of this change? It feels like it only introduces new incompatibilities and a huge complexity for the repos's maintainers to face, but with little benefit. It makes me a bit anxious that more Resolver changes might come and screw my code beyond recovery.

Edit: After reading the PR and the discussion it seems that there's it's somehow related to resolver.layout=follow-schema vs resolver.layout=single-file, but I cannot find any information about what any of these settings mean, and from the PR it reads like the changes only apply for single-file but not for follow-schema. single-file seems to be the default, in opposite to what the documentation suggests. No clue how follow-schema works though, it doesn't seem to generate any resolver file at all with the documentation's setup.

UnAfraid commented 3 weeks ago

Yeah i feel you, behaviour altering changes should be behind configuration option usually opt-in