generateQueryFragments generates invalid plans with missing fragment definitions

.changeset/stupid-lemons-rest.md

@@ -0,0 +1,26 @@
+---
+"@apollo/query-planner": patch
+---
+
+Fix issue with missing fragment definitions due to `generateQueryFragments`.
+
+An incorrect implementation detail in `generateQueryFragments` caused certain queries to be missing fragment definitions. Specifically, subsequent fragment "candidates" with the same type condition and the same length of selections as a previous fragment weren't correctly added to the list of fragments. An example of an affected query is:
+
+```graphql
+query {
+  t {
+    ... on A {
+      x
+      y
+    }
+  }
+  t2 {
+    ... on A {
+      y
+      z
+    }
+  }
+}
+```
+
+In this case, the second selection set would be converted to an inline fragment spread to subgraph fetches, but the fragment definition would be missing.

internals-js/src/operations.ts

@@ -1554,6 +1554,7 @@ export class SelectionSet {
         // compute and handle collisions as necessary.
         const mockHashCode = `on${selection.element.typeCondition}` + selection.selectionSet.selections().length;
         const equivalentSelectionSetCandidates = seenSelections.get(mockHashCode);
+
         if (equivalentSelectionSetCandidates) {
           // See if any candidates have an equivalent selection set, i.e. {x y} and {y x}.
           const match = equivalentSelectionSetCandidates.find(([candidateSet]) => candidateSet.equals(selection.selectionSet!));
@@ -1572,13 +1573,13 @@ export class SelectionSet {
           `_generated_${mockHashCode}_${equivalentSelectionSetCandidates?.length ?? 0}`,
           selection.element.typeCondition
         ).setSelectionSet(minimizedSelectionSet);
+        namedFragments.add(fragmentDefinition);
 
         // Create a new "hash code" bucket or add to the existing one.
-        if (!equivalentSelectionSetCandidates) {
-          seenSelections.set(mockHashCode, [[selection.selectionSet, fragmentDefinition]]);
-          namedFragments.add(fragmentDefinition);
-        } else {
+        if (equivalentSelectionSetCandidates) {
           equivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]);
+        } else {
+            seenSelections.set(mockHashCode, [[selection.selectionSet, fragmentDefinition]]);
         }
 
         return new FragmentSpreadSelection(this.parentType, namedFragments, fragmentDefinition, []);

query-planner-js/src/__tests__/buildPlan.test.ts

@@ -5215,6 +5215,7 @@ describe('Fragment autogeneration', () => {
       type A {
         x: Int
         y: Int
+        z: Int
         t: T
       }
 
@@ -5425,6 +5426,60 @@ describe('Fragment autogeneration', () => {
       }
     `);
   });
+
+  it('fragments that share a hash but are not identical generate their own fragment definitions', () => {
+    const [api, queryPlanner] = composeAndCreatePlannerWithOptions([subgraph], {
+      generateQueryFragments: true,
+    });
+    const operation = operationFromDocument(
+      api,
+      gql`
+        query {
+          t {
+            ... on A {
+              x
+              y
+            }
+          }
+          t2 {
+            ... on A {
+              y
+              z
+            }
+          }
+        }
+      `,
+    );
+
+    const plan = queryPlanner.buildQueryPlan(operation);
+
+    expect(plan).toMatchInlineSnapshot(`
+      QueryPlan {
+        Fetch(service: "Subgraph1") {
+          {
+            t {
+              __typename
+              ..._generated_onA2_0
+            }
+            t2 {
+              __typename
+              ..._generated_onA2_1
+            }
+          }
+
+          fragment _generated_onA2_0 on A {
+            x
+            y
+          }
+
+          fragment _generated_onA2_1 on A {
+            y
+            z
+          }
+        },
+      }
+    `);
+  });
 });
 
 test('works with key chains', () => {