Repository: Khan/genqlient
Pull request #323 (2024-02-26 19:02:54)

Fix non-deterministic generated code involving interfaces and fragments.
In #209, @vikstrous2 added a bugfix to sort the list of interfaces when generating code, so the generated output was stable.  This worked great, but there was one more case where we need to do that, involving interfaces in fragments.

I have:
- [x] Written a clear PR title and description (above)
- [x] Signed the [Khan Academy CLA](https://www.khanacademy.org/r/cla)
- [x] Added tests covering my changes, if applicable
- [ ] Included a link to the issue fixed, if applicable
- [ ] Included documentation, for new features
- [x] Added an entry to the changelog



Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index ab9b287..6201643 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -36,8 +36,9 @@ Note that genqlient is now tested from Go 1.20 through Go 1.22.
 ### Bug fixes:
 - The presence of negative pointer directives, i.e., `# @genqlient(pointer: false)` are now respected even in the when `optional: pointer` is set in the configuration file.
 - Made name collisions between query/mutation arguments and local function variables less likely.
-- Fix generation issue related to golang type implementation of complex graphql union fragments
+- Fix generation issue related to golang type implementation of complex graphql union fragments.
 - Bind correctly to types in the same package as the generated code.
+- Fix non-deterministic generated code when querying graphql interfaces via named fragments.
 
 ## v0.6.0
 
diff --git a/generate/convert.go b/generate/convert.go
index b7e6a6d..3e3dc03 100644
--- a/generate/convert.go
+++ b/generate/convert.go
@@ -841,6 +841,8 @@ func (g *generator) convertNamedFragment(fragment *ast.FragmentDefinition) (goTy
 		return goType, nil
 	case ast.Interface, ast.Union:
 		implementationTypes := g.schema.GetPossibleTypes(typ)
+		// Make sure we generate stable output by sorting the types by name when we get them
+		sort.Slice(implementationTypes, func(i, j int) bool { return implementationTypes[i].Name < implementationTypes[j].Name })
 		goType := &goInterfaceType{
 			GoName:          fragment.Name,
 			SharedFields:    fields,


Test patch:
diff --git a/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go b/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go
index 7888b2e..b7fa016 100644
--- a/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go
+++ b/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go
@@ -273,8 +273,8 @@ func (v *ComplexNamedFragmentsWithInlineUnionUser) __premarshalJSON() (*__premar
 //
 // ContentFields is implemented by the following types:
 // ContentFieldsArticle
-// ContentFieldsVideo
 // ContentFieldsTopic
+// ContentFieldsVideo
 type ContentFields interface {
 	implementsGraphQLInterfaceContentFields()
 	// GetName returns the interface-field "name" from its implementation.
@@ -284,8 +284,8 @@ type ContentFields interface {
 }
 
 func (v *ContentFieldsArticle) implementsGraphQLInterfaceContentFields() {}
-func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields()   {}
 func (v *ContentFieldsTopic) implementsGraphQLInterfaceContentFields()   {}
+func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields()   {}
 
 func __unmarshalContentFields(b []byte, v *ContentFields) error {
 	if string(b) == "null" {
@@ -304,12 +304,12 @@ func __unmarshalContentFields(b []byte, v *ContentFields) error {
 	case "Article":
 		*v = new(ContentFieldsArticle)
 		return json.Unmarshal(b, *v)
-	case "Video":
-		*v = new(ContentFieldsVideo)
-		return json.Unmarshal(b, *v)
 	case "Topic":
 		*v = new(ContentFieldsTopic)
 		return json.Unmarshal(b, *v)
+	case "Video":
+		*v = new(ContentFieldsVideo)
+		return json.Unmarshal(b, *v)
 	case "":
 		return fmt.Errorf(
 			"response was missing Content.__typename")
@@ -331,20 +331,20 @@ func __marshalContentFields(v *ContentFields) ([]byte, error) {
 			*ContentFieldsArticle
 		}{typename, v}
 		return json.Marshal(result)
-	case *ContentFieldsVideo:
-		typename = "Video"
+	case *ContentFieldsTopic:
+		typename = "Topic"
 
 		result := struct {
 			TypeName string `json:"__typename"`
-			*ContentFieldsVideo
+			*ContentFieldsTopic
 		}{typename, v}
 		return json.Marshal(result)
-	case *ContentFieldsTopic:
-		typename = "Topic"
+	case *ContentFieldsVideo:
+		typename = "Video"
 
 		result := struct {
 			TypeName string `json:"__typename"`
-			*ContentFieldsTopic
+			*ContentFieldsVideo
 		}{typename, v}
 		return json.Marshal(result)
 	case nil:
diff --git a/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go b/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go
index c05e913..76499bc 100644
--- a/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go
+++ b/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go
@@ -30,8 +30,8 @@ func (v *ChildVideoFields) GetName() string { return v.Name }
 //
 // ContentFields is implemented by the following types:
 // ContentFieldsArticle
-// ContentFieldsVideo
 // ContentFieldsTopic
+// ContentFieldsVideo
 type ContentFields interface {
 	implementsGraphQLInterfaceContentFields()
 	// GetName returns the interface-field "name" from its implementation.
@@ -41,8 +41,8 @@ type ContentFields interface {
 }
 
 func (v *ContentFieldsArticle) implementsGraphQLInterfaceContentFields() {}
-func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields()   {}
 func (v *ContentFieldsTopic) implementsGraphQLInterfaceContentFields()   {}
+func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields()   {}
 
 func __unmarshalContentFields(b []byte, v *ContentFields) error {
 	if string(b) == "null" {
@@ -61,12 +61,12 @@ func __unmarshalContentFields(b []byte, v *ContentFields) error {
 	case "Article":
 		*v = new(ContentFieldsArticle)
 		return json.Unmarshal(b, *v)
-	case "Video":
-		*v = new(ContentFieldsVideo)
-		return json.Unmarshal(b, *v)
 	case "Topic":
 		*v = new(ContentFieldsTopic)
 		return json.Unmarshal(b, *v)
+	case "Video":
+		*v = new(ContentFieldsVideo)
+		return json.Unmarshal(b, *v)
 	case "":
 		return fmt.Errorf(
 			"response was missing Content.__typename")
@@ -88,20 +88,20 @@ func __marshalContentFields(v *ContentFields) ([]byte, error) {
 			*ContentFieldsArticle
 		}{typename, v}
 		return json.Marshal(result)
-	case *ContentFieldsVideo:
-		typename = "Video"
+	case *ContentFieldsTopic:
+		typename = "Topic"
 
 		result := struct {
 			TypeName string `json:"__typename"`
-			*ContentFieldsVideo
+			*ContentFieldsTopic
 		}{typename, v}
 		return json.Marshal(result)
-	case *ContentFieldsTopic:
-		typename = "Topic"
+	case *ContentFieldsVideo:
+		typename = "Video"
 
 		result := struct {
 			TypeName string `json:"__typename"`
-			*ContentFieldsTopic
+			*ContentFieldsVideo
 		}{typename, v}
 		return json.Marshal(result)
 	case nil: