From 72c02981423742840585ca02a76f0804deaadda3 Mon Sep 17 00:00:00 2001
From: Chris Lenfest <clenfest@apollographql.com>
Date: Mon, 1 Aug 2022 10:43:36 -0500
Subject: [PATCH 1/2] Fix bug in mergeDescriptions, and remove type assertion
 from `otherElementsPrinter`

---
 composition-js/src/__tests__/compose.test.ts | 38 ++++++++++++++++++++
 composition-js/src/merging/merge.ts          | 14 +++++---
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts
index 63cd15460..467bae2e7 100644
--- a/composition-js/src/__tests__/compose.test.ts
+++ b/composition-js/src/__tests__/compose.test.ts
@@ -221,6 +221,44 @@ describe('composition', () => {
     `);
   })
 
+  it('no hint raised when merging empty description', () => {
+    const subgraph1 = {
+      name: 'Subgraph1',
+      typeDefs: gql`
+        schema {
+          query: Query
+        }
+
+        "Type T"
+        type T {
+          a: String @shareable
+        }
+
+        type Query {
+          "Returns tea"
+          t(
+            "An argument that is very important"
+            x: String!
+          ): T
+        }
+      `
+    }
+
+    const subgraph2 = {
+      name: 'Subgraph2',
+      typeDefs: gql`
+        "Type T"
+        type T {
+          a: String @shareable
+        }
+      `
+    }
+
+    const result = composeAsFed2Subgraphs([subgraph1, subgraph2]);
+    assertCompositionSuccess(result);
+    expect(result.hints).toEqual([]);
+  });
+
   it('include types from different subgraphs', () => {
     const subgraphA = {
       typeDefs: gql`
diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts
index 0d51f3cec..0ee44319a 100644
--- a/composition-js/src/merging/merge.ts
+++ b/composition-js/src/merging/merge.ts
@@ -575,7 +575,7 @@ class Merger {
     subgraphElements: (TMismatched | undefined)[],
     elementToString: (elt: TMismatched, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
-    otherElementsPrinter: (elt: string | undefined, subgraphs: string) => string,
+    otherElementsPrinter: (elt: string, subgraphs: string) => string,
     ignorePredicate?: (elt: TMismatched | undefined) => boolean,
     includeMissingSources?: boolean,
     noEndOfMessageDot?: boolean
@@ -604,7 +604,7 @@ class Merger {
     subgraphElements: (TMismatched | undefined)[],
     mismatchAccessor: (element: TMismatched, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
-    otherElementsPrinter: (elt: string | undefined, subgraphs: string) => string,
+    otherElementsPrinter: (elt: string, subgraphs: string) => string,
     reporter: (distribution: string[], astNode: SubgraphASTNode[]) => void,
     ignorePredicate?: (elt: TMismatched | undefined) => boolean,
     includeMissingSources: boolean = false
@@ -637,7 +637,7 @@ class Merger {
       if (v === supergraphMismatch) {
         continue;
       }
-      distribution.push(otherElementsPrinter(v === '' ? undefined : v, printSubgraphNames(names)));
+      distribution.push(otherElementsPrinter(v, printSubgraphNames(names)));
     }
     reporter(distribution, astNodes);
   }
@@ -692,8 +692,12 @@ class Merger {
     }
 
     if (descriptions.length > 0) {
+      // we don't want to raise a hint if a description is ""
+      const nonEmptyDescriptions = descriptions.filter(desc => desc !== '');
       if (descriptions.length === 1) {
         dest.description = descriptions[0];
+      } else if (nonEmptyDescriptions.length === 1) {
+        dest.description = nonEmptyDescriptions[0];
       } else {
         const idx = indexOfMax(counts);
         dest.description = descriptions[idx];
@@ -712,7 +716,7 @@ class Merger {
           subgraphElements: sources,
           elementToString: elt => elt.description,
           supergraphElementPrinter: (desc, subgraphs) => `The supergraph will use description (from ${subgraphs}):\n${descriptionString(desc, '  ')}`,
-          otherElementsPrinter: (desc, subgraphs) => `\nIn ${subgraphs}, the description is:\n${descriptionString(desc!, '  ')}`,
+          otherElementsPrinter: (desc: string, subgraphs) => `\nIn ${subgraphs}, the description is:\n${descriptionString(desc, '  ')}`,
           ignorePredicate: elt => elt?.description === undefined,
           noEndOfMessageDot: true,  // Skip the end-of-message '.' since it would look ugly in that specific case
         });
@@ -2051,7 +2055,7 @@ class Merger {
       // When non-repeatable, we use a similar strategy than for descriptions: we count the occurence of each _different_ application (different arguments)
       // and if there is more than one option (that is, if not all subgraph have the same application), we use in the supergraph whichever application appeared
       // in the most subgraph and warn that we have had to ignore some other applications (of course, if the directive has no arguments, this is moot and
-      // we'll never warn, but this is handled by the general code below. 
+      // we'll never warn, but this is handled by the general code below.
       const differentApplications: Directive[] = [];
       const counts: number[] = [];
       for (const source of perSource) {

From f25f5e479263073376c8328c6a2e8afa78d5b998 Mon Sep 17 00:00:00 2001
From: Chris Lenfest <clenfest@apollographql.com>
Date: Mon, 1 Aug 2022 10:58:18 -0500
Subject: [PATCH 2/2] fix description in testcase

---
 composition-js/src/__tests__/compose.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts
index 467bae2e7..07b282339 100644
--- a/composition-js/src/__tests__/compose.test.ts
+++ b/composition-js/src/__tests__/compose.test.ts
@@ -229,7 +229,7 @@ describe('composition', () => {
           query: Query
         }
 
-        "Type T"
+        ""
         type T {
           a: String @shareable
         }