Don't put join__enumValue onto an EnumValue if all subgraphs who have…

… the enum have the value (#3054)

.changeset/breezy-rocks-crash.md

@@ -0,0 +1,5 @@
+---
+"@apollo/composition": patch
+---
+
+Don't put join\_\_enumValue onto an EnumValue if all subgraphs who have the enum have the value

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

@@ -44,6 +44,11 @@ describe('composition', () => {
         }
 
         union U = S | T
+        
+        enum AnotherEnum {
+          E1
+          E2
+        }
       `
     }
 
@@ -61,6 +66,12 @@ describe('composition', () => {
           V1
           V2
         }
+        
+        enum AnotherEnum {
+          E1
+          E2
+          E3
+        }
       `
     }
 
@@ -91,11 +102,20 @@ describe('composition', () => {
 
       directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
 
+      enum AnotherEnum
+        @join__type(graph: SUBGRAPH1)
+        @join__type(graph: SUBGRAPH2)
+      {
+        E1
+        E2
+        E3 @join__enumValue(graph: SUBGRAPH2)
+      }
+
       enum E
         @join__type(graph: SUBGRAPH2)
       {
-        V1 @join__enumValue(graph: SUBGRAPH2)
-        V2 @join__enumValue(graph: SUBGRAPH2)
+        V1
+        V2
       }
 
       input join__ContextArgument {
@@ -161,6 +181,12 @@ describe('composition', () => {
 
     const [_, api] = schemas(result);
     expect(printSchema(api)).toMatchString(`
+      enum AnotherEnum {
+        E1
+        E2
+        E3
+      }
+
       enum E {
         V1
         V2
@@ -255,8 +281,8 @@ describe('composition', () => {
       enum E
         @join__type(graph: SUBGRAPH2)
       {
-        V1 @join__enumValue(graph: SUBGRAPH2)
-        V2 @join__enumValue(graph: SUBGRAPH2)
+        V1
+        V2
       }
 
       input join__ContextArgument {

composition-js/src/merging/merge.ts

@@ -2213,8 +2213,10 @@ class Merger {
         }
       }
     }
+
+    const sourcesWithEnum = sources.filter(s => s !== undefined).length;
     for (const value of dest.values) {
-      this.mergeEnumValue(sources, dest, value, usage);
+      this.mergeEnumValue(sources, dest, value, usage, sourcesWithEnum);
     }
 
     // We could be left with an enum type with no values, and that's invalid in graphQL
@@ -2231,6 +2233,7 @@ class Merger {
     dest: EnumType,
     value: EnumValue,
     { position, examples }: EnumTypeUsage,
+    sourcesWithEnum: number, // count of how many subgraphs have an enum. Allows us to skip join__EnumValue if all subgraphs who have the enum also have the value
   ) {
     // We merge directives (and description while at it) on the value even though we might remove it later in that function,
     // but we do so because:
@@ -2239,7 +2242,7 @@ class Merger {
     const valueSources = sources.map(s => s?.value(value.name));
     this.mergeDescription(valueSources, value);
     this.recordAppliedDirectivesToMerge(valueSources, value);
-    this.addJoinEnumValue(valueSources, value);
+    this.addJoinEnumValue(valueSources, value, sourcesWithEnum);
 
     const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name);
     const isInaccessible = inaccessibleInSupergraph && value.hasAppliedDirective(inaccessibleInSupergraph.definition);
@@ -2290,7 +2293,11 @@ class Merger {
     }
   }
 
-  private addJoinEnumValue(sources: (EnumValue | undefined)[], dest: EnumValue) {
+  private addJoinEnumValue(sources: (EnumValue | undefined)[], dest: EnumValue, sourcesWithEnumValue: number) {
+    // if all subgraphs who have the enum also have the value, don't bother with @join__enumValue 
+    if (sources.filter(s => s !== undefined).length === sourcesWithEnumValue) {
+      return;
+    }
     const joinEnumValueDirective = this.joinSpec.enumValueDirective(this.merged);
     // We should always be merging with the latest join spec, so this should exists, but well, in prior versions where
     // the directive didn't existed, we simply did had any replacement so ...

gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts

@@ -149,7 +149,7 @@ describe('lifecycle hooks', () => {
     // the supergraph (even just formatting differences), this ID will change
     // and this test will have to updated.
     expect(secondCall[0]!.compositionId).toMatchInlineSnapshot(
-      `"6dc1bde2b9818fabec62208c5d8825abaa1bae89635fa6f3a5ffea7b78fc6d82"`,
+      `"522c6ad4638c2b8d6f2139f176b9c82c3e9ccd82cc0bdcab4e811d0781eb4c16"`,
     );
     // second call should have previous info in the second arg
     expect(secondCall[1]!.compositionId).toEqual(expectedFirstId);

internals-js/src/__tests__/extractSubgraphsFromSupergraph.test.ts

@@ -927,3 +927,106 @@ type U
   field(a: String @federation__fromContext(field: "$context { prop }")): Int!
 }`);
 });
+
+test('no join__enumValue on an enum means it appears in all subgraphs', () => {
+  const supergraph = `
+  schema
+  @link(url: "https://specs.apollo.dev/link/v1.0")
+  @link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION)
+  @link(url: "https://specs.apollo.dev/context/v0.1")
+{
+  query: Query
+}
+
+directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
+
+directive @join__graph(name: String!, url: String!) on ENUM_VALUE
+
+directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
+
+directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
+
+directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
+
+directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
+
+directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
+
+directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
+
+directive @context(name: String!) repeatable on INTERFACE | OBJECT
+
+directive @context__fromContext(field: String) on ARGUMENT_DEFINITION
+
+enum link__Purpose {
+  """
+  \`SECURITY\` features provide metadata necessary to securely resolve fields.
+  """
+  SECURITY
+
+  """
+  \`EXECUTION\` features provide metadata necessary for operation execution.
+  """
+  EXECUTION
+}
+
+scalar link__Import
+
+enum join__Graph {
+  A @join__graph(name: "A", url: "")
+  B @join__graph(name: "B", url: "")
+}
+
+scalar join__FieldSet
+
+scalar join__DirectiveArguments
+
+scalar join__FieldValue
+
+input join__ContextArgument {
+  name: String!
+  type: String!
+  context: String!
+  selection: join__FieldValue!
+}
+
+scalar context__context
+
+type Query
+  @join__type(graph: A)
+  @join__type(graph: B)
+{
+  a: anEnum! @join__field(graph: A)
+  b: anEnum! @join__field(graph: B)
+}
+
+    enum anEnum 
+      @join__type(graph: A)
+      @join__type(graph: B)
+    {
+      ONE @join__enumValue(graph: A) @join__enumValue(graph: B)
+      TWO @join__enumValue(graph: A)
+      THREE   
+    }
+  `;
+
+  const subgraphs = Supergraph.build(supergraph).subgraphs();
+  expect(subgraphs.size()).toBe(2);
+  const [a,b] = subgraphs.values().map((s) => s.schema);
+  const aEnum = a.type('anEnum') as any;
+  expect(aEnum).toBeDefined();
+  expect(aEnum?.kind).toBe('EnumType');
+  expect(aEnum.values.length).toBe(3);
+  expect(aEnum.values[0].name).toBe('ONE');
+  expect(aEnum.values[1].name).toBe('TWO');
+  expect(aEnum.values[2].name).toBe('THREE');
+  expect(b.type('anEnum')).toBeDefined();
+
+  const bEnum = b.type('anEnum') as any;
+  expect(bEnum).toBeDefined();
+  expect(bEnum?.kind).toBe('EnumType');
+  expect(bEnum.values.length).toBe(2);
+  expect(bEnum.values[0].name).toBe('ONE');
+  expect(bEnum.values[1].name).toBe('THREE');
+  expect(b.type('anEnum')).toBeDefined();
+});