From df5eb3cb0e2b4802fcd425ab9c23714de2707db3 Mon Sep 17 00:00:00 2001
From: Dariusz Kuc <9501705+dariuszkuc@users.noreply.github.com>
Date: Mon, 30 Sep 2024 12:50:54 -0500
Subject: [PATCH] fix: fix recursion logic in minimizeSelectionSet (#3158)

When generating fragments we were only "minimizing" subselections for
fields and inline fragments that could be extracted. If inline fragment
cannot be replaced with a fragment spread, we should still minimize its
selection set as it potentially can be optimized as well.
---
 .changeset/violet-cows-talk.md                |  5 ++
 internals-js/src/__tests__/operations.test.ts | 78 ++++++++++++++++++-
 internals-js/src/operations.ts                |  8 +-
 3 files changed, 86 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/violet-cows-talk.md

diff --git a/.changeset/violet-cows-talk.md b/.changeset/violet-cows-talk.md
new file mode 100644
index 000000000..28fb9bcd9
--- /dev/null
+++ b/.changeset/violet-cows-talk.md
@@ -0,0 +1,5 @@
+---
+"@apollo/federation-internals": patch
+---
+
+Fix fragment generation recursion logic to apply minification on all subselections.
diff --git a/internals-js/src/__tests__/operations.test.ts b/internals-js/src/__tests__/operations.test.ts
index 28c048c9e..22fdf67ac 100644
--- a/internals-js/src/__tests__/operations.test.ts
+++ b/internals-js/src/__tests__/operations.test.ts
@@ -90,7 +90,6 @@ describe('generate query fragments', () => {
     );
 
     const withGeneratedFragments = operation.generateQueryFragments();
-    console.log(withGeneratedFragments.toString());
     expect(withGeneratedFragments.toString()).toMatchString(`
       fragment _generated_onB1_0 on B {
         ... on B {
@@ -113,6 +112,83 @@ describe('generate query fragments', () => {
       }
     `);
   });
+
+  test('minimizes all sub selections', () => {
+    const schema = parseSchema(`
+      type Query {
+        foo: A
+      }
+
+      type A {
+        a1: Int
+        a2: I
+      }
+      
+      interface I {
+        i: Int
+      }
+
+      type B implements I {
+        i: Int
+        b1: String
+        b2: I
+      }
+      
+      type C implements I {
+        i: Int
+        c1: Int
+        c2: String
+      }
+    `);
+
+    const operation = parseOperation(
+      schema,
+      `
+      query Foo($flag: Boolean!) {
+        foo {
+          a1
+          a2 {
+            i
+            ... on B @include(if: $flag) {
+              b1
+              b2 {
+                i
+                ... on C {
+                  c1
+                  c2
+                }
+              }
+            }
+          }
+        }
+      }
+    `,
+    );
+
+    const withGeneratedFragments = operation.generateQueryFragments();
+    expect(withGeneratedFragments.toString()).toMatchString(`
+      fragment _generated_onC2_0 on C {
+        c1
+        c2
+      }
+    
+      query Foo($flag: Boolean!) {
+        foo {
+          a1
+          a2 {
+            i
+            ... on B @include(if: $flag) {
+              b1
+              b2 {
+                i
+                ..._generated_onC2_0
+              }
+            }
+          }
+        }
+      }
+    `);
+  });
 });
 
 describe('fragments optimization', () => {
diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts
index 6bc6a65ce..ea2bf3eb4 100644
--- a/internals-js/src/operations.ts
+++ b/internals-js/src/operations.ts
@@ -1658,10 +1658,10 @@ export class SelectionSet {
         }
 
         return new FragmentSpreadSelection(this.parentType, namedFragments, fragmentDefinition, []);
-      } else if (selection.kind === 'FieldSelection') {
-        if (selection.selectionSet) {
-          selection = selection.withUpdatedSelectionSet(selection.selectionSet.minimizeSelectionSet(namedFragments, seenSelections)[0]);
-        }
+      }
+
+      if (selection.selectionSet) {
+        selection = selection.withUpdatedSelectionSet(selection.selectionSet.minimizeSelectionSet(namedFragments, seenSelections)[0]);
       }
       return selection;
     });