Gateway over-merging unions cleanup (apollographql/apollo-server#3616)

This commit cleans up and simplifies the approach taken by apollographql/apollo-server#3581
to prevent the gateway from over-merging fields of unioned types.

The correct information and groupings were already present in the
affected code, they just needed to be utilized correctly. Specifically,
the internal for loop yielding `fieldsForParentType` was not being
used - the less specifically grouped `fieldsForResponseName` was.

Co-authored-by: Tejas Shikhare <tshikhare@netflix.com>
Apollo-Orig-Commit-AS: apollographql/apollo-server@61e6941

gateway-js/src/FieldSet.ts

@@ -64,25 +64,8 @@ function groupBy<T, U>(keyFunction: (element: T) => U) {
   };
 }
 
-// The response name isn't sufficient for determining uniqueness. In the case of
-// unions, for example, we can see a response name collision where the parent type
-// is different. In this case, these should not be merged (media)!
-// query {
-//   content {
-//     ... on Audio {
-//       media {
-//         url
-//       }
-//     }
-//     ... on Video {
-//       media {
-//         aspectRatio
-//       }
-//     }
-//   }
-// }
-export const groupByParentTypeAndResponseName = groupBy<Field, string>(field =>
-  `${field.scope.parentType}:${getResponseName(field.fieldNode)}`,
+export const groupByResponseName = groupBy<Field, string>(field =>
+  getResponseName(field.fieldNode)
 );
 
 export const groupByParentType = groupBy<Field, GraphQLCompositeType>(
@@ -98,7 +81,7 @@ export function selectionSetFromFieldSet(
     selections: Array.from(groupByParentType(fields)).flatMap(
       ([typeCondition, fieldsByParentType]: [GraphQLCompositeType, FieldSet]) =>
         wrapInInlineFragmentIfNeeded(
-          Array.from(groupByParentTypeAndResponseName(fieldsByParentType).values()).map(
+          Array.from(groupByResponseName(fieldsByParentType).values()).map(
             fieldsByResponseName => {
               return combineFields(fieldsByResponseName)
                 .fieldNode;

gateway-js/src/buildQueryPlan.ts

@@ -31,7 +31,7 @@ import {
   Field,
   FieldSet,
   groupByParentType,
-  groupByParentTypeAndResponseName,
+  groupByResponseName,
   matchesField,
   selectionSetFromFieldSet,
   Scope,
@@ -373,7 +373,7 @@ function splitFields(
   fields: FieldSet,
   groupForField: (field: Field<GraphQLObjectType>) => FetchGroup,
 ) {
-  for (const fieldsForResponseName of groupByParentTypeAndResponseName(fields).values()) {
+  for (const fieldsForResponseName of groupByResponseName(fields).values()) {
     for (const [parentType, fieldsForParentType] of groupByParentType(
       fieldsForResponseName,
     )) {
@@ -413,7 +413,7 @@ function splitFields(
             scope as Scope<typeof parentType>,
             group,
             path,
-            fieldsForResponseName,
+            fieldsForParentType,
           ),
         );
       } else {
@@ -452,7 +452,7 @@ function splitFields(
               field.fieldNode,
             );
 
-            const fieldsWithRuntimeParentType = fieldsForResponseName.map(field => ({
+            const fieldsWithRuntimeParentType = fieldsForParentType.map(field => ({
               ...field,
               fieldDef,
             }));