Improve composition hints around types that are strictly @external (#…

…2951)

Note: 1st commit highlights the issue with a test case, 2nd commit
proposes the change + test update. No existing tests are affected.

Composition currently gives unhelpful hints around inconsistent value
types which are only defined for the sake of referencing (all fields or
type is marked `@external`).

In the example test case, the `Permissions` type is only _referenced_ in
the `account` (2nd) subgraph due to the `@requires`, so a partial
definition seems reasonable here. The hint suggests adding the missing
field in order to make the type consistent across subgraphs.

However, if we take the actionable step recommended by that hint and add
the missing field (and mark it `@external`, by necessity), we end up
with a composition error instead:
> [Subgraph2] Field "Permissions.canEdit" is marked @external but is not
used in any federation directive (@key, @provides, @requires) or to
satisfy an interface; the field declaration has no use and should be
removed (or the field should not be @external).

.changeset/weak-bottles-crash.md

@@ -0,0 +1,5 @@
+---
+"@apollo/composition": patch
+---
+
+Stop emitting "inconsistent value type" hints against definitions where the type is marked `@external` or all fields are marked `@external`.

composition-js/src/__tests__/hints.test.ts

@@ -1353,4 +1353,86 @@ describe('when a directive causes an implicit federation version upgrade', () =>
     assertCompositionSuccess(result);
     expect(result).toNotRaiseHints();
   });
-})
+});
+
+describe('when a partially-defined type is marked @external or all fields are marked @external', () => {
+  describe('value types', () => {
+    it('with type marked @external', () => {
+      const meSubgraph = gql`
+        type Query {
+          me: Account
+        }
+
+        type Account @key(fields: "id") {
+          id: ID!
+          name: String
+          permissions: Permissions
+        }
+
+        type Permissions {
+          canView: Boolean
+          canEdit: Boolean
+        }
+      `;
+
+      const accountSubgraph = gql`
+        type Query {
+          account: Account
+        }
+
+        type Account @key(fields: "id") {
+          id: ID!
+          permissions: Permissions @external
+          isViewer: Boolean @requires(fields: "permissions { canView }")
+        }
+
+        type Permissions @external {
+          canView: Boolean
+        }
+      `;
+
+      const result = mergeDocuments(meSubgraph, accountSubgraph);
+      expect(result).toNotRaiseHints();
+    });
+
+    it('with all fields marked @external', () => {
+      const meSubgraph = gql`
+        type Query {
+          me: Account
+        }
+
+        type Account @key(fields: "id") {
+          id: ID!
+          name: String
+          permissions: Permissions
+        }
+
+        type Permissions {
+          canView: Boolean
+          canEdit: Boolean
+          canDelete: Boolean
+        }
+      `;
+
+      const accountSubgraph = gql`
+        type Query {
+          account: Account
+        }
+
+        type Account @key(fields: "id") {
+          id: ID!
+          permissions: Permissions @external
+          isViewer: Boolean @requires(fields: "permissions { canView canEdit }")
+        }
+
+        type Permissions {
+          canView: Boolean @external
+          canEdit: Boolean @external
+        }
+      `;
+
+      const result = mergeDocuments(meSubgraph, accountSubgraph);
+      expect(result).toNotRaiseHints();
+    });
+  });
+});

composition-js/src/merging/merge.ts

@@ -965,9 +965,9 @@ class Merger {
         typeDescription = 'interface'
         break;
     }
-    for (const source of sources) {
+    for (const [index, source] of sources.entries()) {
       // As soon as we find a subgraph that has the type but not the field, we hint.
-      if (source && !source.field(field.name)) {
+      if (source && !source.field(field.name) && !this.areAllFieldsExternal(index, source)) {
         this.mismatchReporter.reportMismatchHint({
           code: hintId,
           message: `Field "${field.coordinate}" of ${typeDescription} type "${dest}" is defined in some but not all subgraphs that define "${dest}": `,
@@ -1081,6 +1081,10 @@ class Merger {
     return this.metadata(sourceIdx).isFieldFullyExternal(field);
   }
 
+  private areAllFieldsExternal(sourceIdx: number, type: ObjectType | InterfaceType): boolean {
+    return type.fields().every(f => this.isExternal(sourceIdx, f));
+  }
+
   private validateAndFilterExternal(sources: (FieldDefinition<any> | undefined)[]): (FieldDefinition<any> | undefined)[] {
     const filtered: (FieldDefinition<any> | undefined)[] = [];
     for (const [i, source] of sources.entries()) {