search

Khan / genqlient

a truly type-safe Go GraphQL client
MIT License
1.09k stars 113 forks source link

move genqlient Go module to 1.20 #318

Closed StevenACoffman closed 9 months ago

StevenACoffman commented 9 months ago

gqlgen's module supports Go 1.20+, and I suggest that genqlient should also do so.

Go 1.20 was released more than 1 year ago (01 Feb 2023), and support ended on 06 Feb 2024. Each major Go release is supported until there are two newer major releases.

In the past, we have only prolonged gqlgen/genqlient support for older Go when AppEngine Go was mired in the past.

Currently, Appengine no longer supports Go 1.18, and supports up to Go 1.21.

I will wait for possible objections for a few days before merging this PR. I also expect that we would want to cut a new release of genqlient either before or after merging this PR.

Signed-off-by: Steve Coffman steve@khanacademy.org

benjaminjkraft commented 9 months ago

Any reason to drop support? In general my approach has been to drop support for old versions only when there's a reason to (e.g. dropping 1.17 to get generics). If there is I agree it's fine, but until such time why not support more?

StevenACoffman commented 9 months ago

I'd like to keep genqlient and gqlgen supporting similar versions. While I've never actively tried to break Go 1.18 compatibility, gqlgen has had to move the floor to 1.20. Right now, Go 1.18 users of genqlient who consume the latest version of gqlgen (and genqlient's other dependencies) will have subtle problems that are hard to diagnose.

In another 6 months, when Go 1.21+ is widely installed, incrementing Go version in a module should become entirely a non-issue, as Go 1.21 will download a newer toolchain so practically everyone should be able to run any newer version of Go regardless of their platform of choice.

While Go 1.18 technically supports generics, subsequent releases are practically required to do much of interest with them.

In releasing generics in Go 1.18, the Go authors were aware that it was a huge change, so they were fairly conservative in that release. For instance, they put several packages in x/exp repository; so the Go 1 guarantee did not cover their API and they changed as they gained more experience with generics. Various libraries that jumped to use these experimental packages then caused many problems as it was impossible to combine their dependency with other libraries that required different versions. Many contributions to gqlgen have had to be rejected because of a transitive dependency mess that has only recently been sorted out in the Go ecosystem by adopting a version of Go where these libraries are just part of the stdlib and are not going to break compatibility.

In addition, there have been several tweaks to generics support like a very small correction to the scope of type parameters in method declarations, and Comparable types (such as ordinary interfaces) may now satisfy comparable constraints, even if the type arguments are not strictly comparable (comparison may panic at runtime). This makes it possible to instantiate a type parameter constrained by comparable (e.g., a type parameter for a user-defined generic map key) with a non-strictly comparable type argument such as an interface type, or a composite type containing an interface type.

People tend to make contributions to gqlgen expecting these behaviors and it has sometimes been hard to tell why they fail in Go 1.18 or Go 1.19 production usage but pass in tests in those environments

See:

benjaminjkraft commented 9 months ago

I wrote a long response about why these reasons are hypothetical and/or don't apply to genqlient. I honestly still don't see a good reason to cut support now. But then I took a deep breath and realized it doesn't cost much either, and it's easier to merge than to discuss further.

Anyway, we should add a changelog entry just in case; you can reword the one at the top I added about testing up to 1.22.

benjaminjkraft commented 9 months ago

BTW, if we want slices, which we do have a few possible callers of (reverse is slices.Reverse[string], a few places could use slices.ContainsFunc, and some possible callers for slices.Sort/cmp as well), we'll need 1.21.

StevenACoffman commented 9 months ago

Thanks for going along with it.

Yeah, I would love to advance all the way to Go 1.21 for the slices package in stdlib, but until it has been a year from its release (and when 1.23 drops) in August, I feel like there may be enough people stuck on 1.20 that it could impact people.

That's what I've been doing with gqlgen at least.