Massive undocumented breaking change in v0.17.50

[Vilsol]

I have hit a major issue due to this undocumented breaking change (#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

• It has not generated any imports for anything except for what it itself is using.
• It has blindly copied the code from within the resolver functions, disregarded the function argument names, therefore completely breaking code that used those variables.

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]

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]

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

[StevenACoffman]

This bad situation is actually both my mistake and my miscommunication combined, and I'm quite sorry.

There was originally a config option enable_rewrite_for_single_file in that PR to opt-in to rewriting resolvers or to preserve existing resolver code even when the schema changed to no longer match.

I suggested that instead the config option be rewrite apply equally to both single-file and follow-schema. My intent was to reduce the growing divergence in the code that places all resolvers in a single file and the code that places the resolvers in a directory layout that matches the schema.

The person who did that PR was an intern under the supervision of another maintainer. That maintainer's current employer is under legal restrictions preventing them from directly contributing, but the maintainer and I had side conversations so I assumed that it had been more carefully vetted so I merged it based on that assessment, rather than doing my own.

That was a big mistake. Again, I'm very sorry.

I'd welcome a PR to add a config option to at least opt-out of this rewrite behavior.

[JonasDoe]

Thanks for the response! If I find time, I can try to create a PR. But it might take a while, so if somebody is more into the topic and faster with adding this option, I'll gladly yield it. ;)

[StevenACoffman]

This appears to illustrate the PR change before the config option was removed:
1855758...d7ffe0c

[StevenACoffman]

I put up a PR for #3359 for your consideration
update: merged

[StevenACoffman]

Please see https://github.com/99designs/gqlgen/releases/tag/v0.17.56 and hopefully people can add preserve_rewrite: true to your gqlgen config and go back to how things were.