From 8ee7b952239904f28c3d8985eeaad9939c5d3b61 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@benjamn.com>
Date: Thu, 27 Jun 2024 14:47:54 -0400
Subject: [PATCH 1/5] Avoid re-filtering NamedType objects in Merger.merge.

---
 composition-js/src/merging/merge.ts | 49 ++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts
index 57e9b618a..e4ffc9d42 100644
--- a/composition-js/src/merging/merge.ts
+++ b/composition-js/src/merging/merge.ts
@@ -57,7 +57,6 @@ import {
   EnumValue,
   baseType,
   isEnumType,
-  filterTypesOfKind,
   isNonNullType,
   isExecutableDirectiveLocation,
   parseFieldSetArgument,
@@ -506,19 +505,48 @@ class Merger {
     this.addTypesShallow();
     this.addDirectivesShallow();
 
-    const typesToMerge = this.merged.types()
-      .filter((type) => !this.linkSpec.isSpecType(type) && !this.joinSpec.isSpecType(type));
+    const objectTypes: ObjectType[] = [];
+    const interfaceTypes: InterfaceType[] = [];
+    const unionTypes: UnionType[] = [];
+    const enumTypes: EnumType[] = [];
+    const nonUnionEnumTypes: NamedType[] = [];
+
+    this.merged.types().forEach(type => {
+      if (
+        this.linkSpec.isSpecType(type) ||
+        this.joinSpec.isSpecType(type)
+      ) {
+        return;
+      }
+
+      switch (type.kind) {
+        case 'UnionType':
+          unionTypes.push(type);
+          return;
+        case 'EnumType':
+          enumTypes.push(type);
+          return;
+        case 'ObjectType':
+          objectTypes.push(type);
+          break;
+        case 'InterfaceType':
+          interfaceTypes.push(type);
+          break;
+      }
+
+      nonUnionEnumTypes.push(type);
+    });
 
     // Then, for object and interface types, we merge the 'implements' relationship, and we merge the unions.
     // We do this first because being able to know if a type is a subtype of another one (which relies on those
     // 2 things) is used when merging fields.
-    for (const objectType of filterTypesOfKind<ObjectType>(typesToMerge, 'ObjectType')) {
+    for (const objectType of objectTypes) {
       this.mergeImplements(this.subgraphsTypes(objectType), objectType);
     }
-    for (const interfaceType of filterTypesOfKind<InterfaceType>(typesToMerge, 'InterfaceType')) {
+    for (const interfaceType of interfaceTypes) {
       this.mergeImplements(this.subgraphsTypes(interfaceType), interfaceType);
     }
-    for (const unionType of filterTypesOfKind<UnionType>(typesToMerge, 'UnionType')) {
+    for (const unionType of unionTypes) {
       this.mergeType(this.subgraphsTypes(unionType), unionType);
     }
 
@@ -528,11 +556,8 @@ class Merger {
     // calling root type a "value type" when hinting).
     this.mergeSchemaDefinition(this.subgraphsSchema.map(s => s.schemaDefinition), this.merged.schemaDefinition);
 
-    for (const type of typesToMerge) {
-      // We've already merged unions above and we've going to merge enums last
-      if (type.kind === 'UnionType' || type.kind === 'EnumType') {
-        continue;
-      }
+    // We've already merged unions above and we've going to merge enums last
+    for (const type of nonUnionEnumTypes) {
       this.mergeType(this.subgraphsTypes(type), type);
     }
 
@@ -547,7 +572,7 @@ class Merger {
     // We merge enum dead last because enums can be used as both input and output types and the merging behavior
     // depends on their usage and it's easier to check said usage if everything else has been merge (at least
     // anything that may use an enum type, so all fields and arguments).
-    for (const enumType of filterTypesOfKind<EnumType>(typesToMerge, 'EnumType')) {
+    for (const enumType of enumTypes) {
       this.mergeType(this.subgraphsTypes(enumType), enumType);
     }
 

From 60ba8709f95c63fec65b1a9840b582d580f4349b Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@benjamn.com>
Date: Tue, 2 Jul 2024 10:06:34 -0400
Subject: [PATCH 2/5] Use sparse Map instead of large sources arrays during
 merge.

The goal of this commit is to switch from using a dense array for the
various 'sources' passed around in mergeType code, to using a Map
instead. These Map structures have the potential to omit many elements
corresponding to irrelevant subgraphs, becoming sparser than an array,
but this commit preserves the dense array-like behavior, for now.
---
 composition-js/src/composeDirectiveManager.ts |   5 +-
 composition-js/src/merging/merge.ts           | 443 ++++++++++++------
 composition-js/src/merging/reporter.ts        |  11 +-
 3 files changed, 296 insertions(+), 163 deletions(-)

diff --git a/composition-js/src/composeDirectiveManager.ts b/composition-js/src/composeDirectiveManager.ts
index e2fe7b170..90fa24fae 100644
--- a/composition-js/src/composeDirectiveManager.ts
+++ b/composition-js/src/composeDirectiveManager.ts
@@ -15,6 +15,7 @@ import {
 import { GraphQLError } from 'graphql';
 import { CompositionHint, HINTS } from './hints';
 import { MismatchReporter } from './merging/reporter';
+import { sourcesFromArray } from './merging';
 
 /**
  * Return true if the directive from the same core feature has a different name in the subgraph
@@ -367,7 +368,7 @@ export class ComposeDirectiveManager {
         this.mismatchReporter.reportMismatchErrorWithoutSupergraph(
           ERRORS.DIRECTIVE_COMPOSITION_ERROR,
           'Composed directive is not named consistently in all subgraphs',
-          this.subgraphs.values()
+          sourcesFromArray(this.subgraphs.values()
             .map(sg => {
               const item = items.find(item => sg.name === item.sgName);
               return item ? {
@@ -384,7 +385,7 @@ export class ComposeDirectiveManager {
                 sourceAST,
                 item: val.item,
               } : undefined;
-            }),
+            })),
           (elt) => elt ? `"@${elt.item.directiveNameAs}"` : undefined
         );
       }
diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts
index e4ffc9d42..8d9baaf16 100644
--- a/composition-js/src/merging/merge.ts
+++ b/composition-js/src/merging/merge.ts
@@ -93,7 +93,45 @@ import { inspect } from "util";
 import { collectCoreDirectivesToCompose, CoreDirectiveInSubgraphs } from "./coreDirectiveCollector";
 import { CompositionOptions } from "../compose";
 
-type FieldOrUndefinedArray = (FieldDefinition<any> | undefined)[];
+export type Sources<T> = Map<number, T | undefined>;
+
+function mapSources<T, R>(
+  sources: Sources<T>,
+  mapper: (source: T | undefined, index: number) => R,
+): Sources<R> {
+  const result: Sources<R> = new Map;
+  sources.forEach((source, idx) => {
+    result.set(idx, mapper(source, idx));
+  });
+  return result;
+}
+
+function filterSources<T>(sources: Sources<T>): Sources<T> {
+  const result: Sources<T> = new Map;
+  sources.forEach((source, idx) => {
+    if (typeof source !== 'undefined') {
+      result.set(idx, source);
+    }
+  });
+  return result;
+}
+
+function someSources<T>(sources: Sources<T>, predicate: (source: T | undefined, index: number) => boolean | undefined): boolean {
+  for (const [idx, source] of sources.entries()) {
+    if (predicate(source, idx)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+export function sourcesFromArray<T>(array: (T | undefined)[]): Sources<T> {
+  const sources: Sources<T> = new Map;
+  array.forEach((source, idx) => {
+    sources.set(idx, source);
+  });
+  return sources;
+}
 
 export type MergeResult = MergeSuccess | MergeFailure;
 
@@ -106,52 +144,59 @@ type FieldMergeContextProperties = {
 
 // for each source, specify additional properties that validate functions can set
 class FieldMergeContext {
-  _props: FieldMergeContextProperties[];
+  _props: Map<number, FieldMergeContextProperties>;
 
-  constructor(sources: unknown[]) {
-    this._props = (
-      new Array(sources.length)).fill(true).map(_ => ({
+  constructor(sources: Sources<FieldDefinition<any> | InputFieldDefinition>) {
+    this._props = new Map;
+    sources.forEach((_, i) => {
+      this._props.set(i, {
         usedOverridden: false,
         unusedOverridden: false,
         overrideWithUnknownTarget: false,
         overrideLabel: undefined,
-      }));
+      });
+    });
   }
 
   isUsedOverridden(idx: number) {
-    return this._props[idx].usedOverridden;
+    return !!this._props.get(idx)?.usedOverridden;
   }
 
   isUnusedOverridden(idx: number) {
-    return this._props[idx].unusedOverridden;
+    return !!this._props.get(idx)?.unusedOverridden;
   }
 
   hasOverrideWithUnknownTarget(idx: number) {
-    return this._props[idx].overrideWithUnknownTarget;
+    return !!this._props.get(idx)?.overrideWithUnknownTarget;
   }
 
   overrideLabel(idx: number) {
-    return this._props[idx].overrideLabel;
+    return this._props.get(idx)?.overrideLabel;
   }
 
   setUsedOverridden(idx: number) {
-    this._props[idx].usedOverridden = true;
+    this._props.get(idx)!.usedOverridden = true;
   }
 
   setUnusedOverridden(idx: number) {
-    this._props[idx].unusedOverridden = true;
+    this._props.get(idx)!.unusedOverridden = true;
   }
 
   setOverrideWithUnknownTarget(idx: number) {
-    this._props[idx].overrideWithUnknownTarget = true;
+    this._props.get(idx)!.overrideWithUnknownTarget = true;
   }
 
   setOverrideLabel(idx: number, label: string) {
-    this._props[idx].overrideLabel = label;
+    this._props.get(idx)!.overrideLabel = label;
   }
 
-  some(predicate: (props: FieldMergeContextProperties) => boolean): boolean {
-    return this._props.some(predicate);
+  some(predicate: (props: FieldMergeContextProperties, index: number) => boolean) {
+    for (const [i, props] of this._props.entries()) {
+      if (predicate(props, i)) {
+        return true;
+      }
+    }
+    return false;
   }
 }
 
@@ -301,7 +346,7 @@ class Merger {
   private mismatchReporter: MismatchReporter;
   private appliedDirectivesToMerge: {
     names: Set<string>,
-    sources: (SchemaElement<any, any> | undefined)[],
+    sources: Sources<SchemaElement<any, any>>,
     dest: SchemaElement<any, any>,
   }[];
   private joinSpec: JoinSpecDefinition;
@@ -437,7 +482,7 @@ class Merger {
           ERRORS.LINK_IMPORT_NAME_MISMATCH,
           `The "@${name}" directive (from ${url}) is imported with mismatched name between subgraphs: it is imported as `,
           directive,
-          this.subgraphs.values().map((s) => definitionsPerSubgraph.get(s.name)),
+          sourcesFromArray(this.subgraphs.values().map((s) => definitionsPerSubgraph.get(s.name))),
           (def) => `"@${def.name}"`,
         );
         return;
@@ -554,7 +599,10 @@ class Merger {
     // we merge types next, we actually rely on this having been called to detect "root types"
     // (in order to skip the _entities and _service fields on that particular type, and to avoid
     // calling root type a "value type" when hinting).
-    this.mergeSchemaDefinition(this.subgraphsSchema.map(s => s.schemaDefinition), this.merged.schemaDefinition);
+    this.mergeSchemaDefinition(
+      sourcesFromArray(this.subgraphsSchema.map(s => s.schemaDefinition)),
+      this.merged.schemaDefinition,
+    );
 
     // We've already merged unions above and we've going to merge enums last
     for (const type of nonUnionEnumTypes) {
@@ -566,7 +614,10 @@ class Merger {
       if (this.linkSpec.isSpecDirective(definition) || this.joinSpec.isSpecDirective(definition)) {
         continue;
       }
-      this.mergeDirectiveDefinition(this.subgraphsSchema.map(s => s.directive(definition.name)), definition);
+      this.mergeDirectiveDefinition(
+        sourcesFromArray(this.subgraphsSchema.map(s => s.directive(definition.name))),
+        definition,
+      );
     }
 
     // We merge enum dead last because enums can be used as both input and output types and the merging behavior
@@ -734,16 +785,16 @@ class Merger {
       ERRORS.TYPE_KIND_MISMATCH,
       `Type "${mismatchedType}" has mismatched kind: it is defined as `,
       supergraphType,
-      this.subgraphsSchema.map(s => s.type(mismatchedType)),
+      sourcesFromArray(this.subgraphsSchema.map(s => s.type(mismatchedType))),
       typeKindToString
     );
   }
 
-  private subgraphsTypes<T extends NamedType>(supergraphType: T): (T | undefined)[] {
-    return this.subgraphs.values().map((subgraph) => {
+  private subgraphsTypes<T extends NamedType>(supergraphType: T): Sources<T> {
+    return sourcesFromArray(this.subgraphs.values().map(subgraph => {
       const type = subgraph.schema.type(supergraphType.name);
       if (!type) {
-        return undefined;
+        return;
       }
 
       // At this point, we have already reported errors for type mismatches (and so composition
@@ -751,13 +802,14 @@ class Merger {
       // that don't have the "proper" kind.
       const kind = subgraph.metadata().isInterfaceObjectType(type) ? 'InterfaceType' : type.kind;
       if (kind !== supergraphType.kind) {
-        return undefined;
+        return;
       }
+
       return type as T;
-    });
+    }));
   }
 
-  private mergeImplements<T extends ObjectType | InterfaceType>(sources: (T | undefined)[], dest: T) {
+  private mergeImplements<T extends ObjectType | InterfaceType>(sources: Sources<T>, dest: T) {
     const implemented = new Set<string>();
     const joinImplementsDirective = this.joinSpec.implementsDirective(this.merged)!;
     for (const [idx, source] of sources.entries()) {
@@ -772,10 +824,10 @@ class Merger {
     implemented.forEach(itf => dest.addImplementedInterface(itf));
   }
 
-  private mergeDescription<T extends SchemaElement<any, any>>(sources: (T | undefined)[], dest: T) {
+  private mergeDescription<T extends SchemaElement<any, any>>(sources: Sources<T>, dest: T) {
     const descriptions: string[] = [];
     const counts: number[] = [];
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source || source.description === undefined) {
         continue;
       }
@@ -830,7 +882,7 @@ class Merger {
   // We could express this through a generic argument, but typescript is not smart enough to save us
   // type-casting even if we do, and in fact, using a generic forces a case on `dest` for some reason.
   // So we don't bother.
-  private mergeType(sources: (NamedType | undefined)[], dest: NamedType) {
+  private mergeType(sources: Sources<NamedType>, dest: NamedType) {
     this.checkForExtensionWithNoBase(sources, dest);
     this.mergeDescription(sources, dest);
     this.addJoinType(sources, dest);
@@ -841,25 +893,25 @@ class Merger {
         // Since we don't handle applied directives yet, we have nothing specific to do for scalars.
         break;
       case 'ObjectType':
-        this.mergeObject(sources as (ObjectType | undefined)[], dest);
+        this.mergeObject(sources as Sources<ObjectType>, dest);
         break;
       case 'InterfaceType':
         // Note that due to @interfaceObject, we can have some ObjectType in the sources, not just interfaces.
-        this.mergeInterface(sources as (InterfaceType | ObjectType | undefined)[], dest);
+        this.mergeInterface(sources as Sources<InterfaceType | ObjectType>, dest);
         break;
       case 'UnionType':
-        this.mergeUnion(sources as (UnionType | undefined)[], dest);
+        this.mergeUnion(sources as Sources<UnionType>, dest);
         break;
       case 'EnumType':
-        this.mergeEnum(sources as (EnumType | undefined)[], dest);
+        this.mergeEnum(sources as Sources<EnumType>, dest);
         break;
       case 'InputObjectType':
-        this.mergeInput(sources as (InputObjectType | undefined)[], dest);
+        this.mergeInput(sources as Sources<InputObjectType>, dest);
         break;
     }
   }
 
-  private checkForExtensionWithNoBase(sources: (NamedType | undefined)[], dest: NamedType) {
+  private checkForExtensionWithNoBase(sources: Sources<NamedType>, dest: NamedType) {
     if (isObjectType(dest) && dest.isRootType()) {
       return;
     }
@@ -890,7 +942,7 @@ class Merger {
     }
   }
 
-  private addJoinType(sources: (NamedType | undefined)[], dest: NamedType) {
+  private addJoinType(sources: Sources<NamedType>, dest: NamedType) {
     const joinTypeDirective = this.joinSpec.typeDirective(this.merged);
     for (const [idx, source] of sources.entries()) {
       if (!source) {
@@ -918,7 +970,7 @@ class Merger {
     }
   }
 
-  private mergeObject(sources: (ObjectType | undefined)[], dest: ObjectType) {
+  private mergeObject(sources: Sources<ObjectType>, dest: ObjectType) {
     const isEntity = this.hintOnInconsistentEntity(sources, dest);
     const isValueType = !isEntity && !dest.isRootType();
     const isSubscription = dest.isSubscriptionRootType();
@@ -934,7 +986,7 @@ class Merger {
         if (isValueType) {
           this.hintOnInconsistentValueTypeField(sources, dest, destField);
         }
-        const subgraphFields = sources.map(t => t?.field(destField.name));
+        const subgraphFields = mapSources(sources, t => t?.field(destField.name));
         const mergeContext = this.validateOverride(subgraphFields, destField);
 
         if (isSubscription) {
@@ -952,10 +1004,10 @@ class Merger {
   }
 
   // Return whether the type is an entity in at least one subgraph.
-  private hintOnInconsistentEntity(sources: (ObjectType | undefined)[], dest: ObjectType): boolean {
+  private hintOnInconsistentEntity(sources: Sources<ObjectType>, dest: ObjectType): boolean {
     const sourceAsEntity: ObjectType[] = [];
     const sourceAsNonEntity: ObjectType[] = [];
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -983,7 +1035,7 @@ class Merger {
 
   // Assume it is called on a field of a value type
   private hintOnInconsistentValueTypeField(
-    sources: (ObjectType | InterfaceType | undefined)[],
+    sources: Sources<ObjectType | InterfaceType>,
     dest: ObjectType | InterfaceType,
     field: FieldDefinition<any>,
   ) {
@@ -1063,7 +1115,13 @@ class Merger {
             // So we validate field sharing now (it's convenient to wait until now as now that
             // the field is part of the supergraph, we can just call `validateFieldSharing` with
             // all sources `undefined` and it wil still find and check the `@interfaceObject`).
-            const sources = new Array<undefined>(this.names.length);
+            const sources: Sources<FieldDefinition<ObjectType>> = new Map;
+            for (let i = 0; i < this.names.length; ++i) {
+              // We don't usually want undefined sources in our Sources maps,
+              // but both validateFieldSharing and FieldMergeContext need the
+              // undefined sources to be registered in order to do their work.
+              sources.set(i, undefined);
+            }
             this.validateFieldSharing(sources, implemField, new FieldMergeContext(sources));
           }
         }
@@ -1091,8 +1149,8 @@ class Merger {
     });
   }
 
-  private addFieldsShallow<T extends ObjectType | InterfaceType | InputObjectType>(sources: (T | undefined)[], dest: T) {
-    for (const source of sources) {
+  private addFieldsShallow<T extends ObjectType | InterfaceType | InputObjectType>(sources: Sources<T>, dest: T) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -1119,15 +1177,15 @@ class Merger {
     return type.fields().every(f => this.isExternal(sourceIdx, f));
   }
 
-  private validateAndFilterExternal(sources: (FieldDefinition<any> | undefined)[]): (FieldDefinition<any> | undefined)[] {
-    const filtered: (FieldDefinition<any> | undefined)[] = [];
+  private validateAndFilterExternal(sources: Sources<FieldDefinition<any>>): Sources<FieldDefinition<any>> {
+    const filtered: Sources<FieldDefinition<any>> = new Map;
     for (const [i, source] of sources.entries()) {
       // If the source doesn't have the field or it is not external, we mirror the input
-      if (source == undefined || !this.isExternal(i, source)) {
-        filtered.push(source);
+      if (!source || !this.isExternal(i, source)) {
+        filtered.set(i, source);
       } else {
         // Otherwise, we filter out the source, but also "validate" it.
-        filtered.push(undefined);
+        filtered.set(i, undefined);
 
         // We don't allow "merged" directives on external fields because as far as merging goes, external fields don't really
         // exists and allowing "merged" directives on them is dodgy. To take examples, having a `@deprecated` or `@tag` on
@@ -1153,8 +1211,13 @@ class Merger {
     return filtered;
   }
 
-  private hasExternal(sources: FieldOrUndefinedArray): boolean {
-    return sources.some((s, i) => s !== undefined && this.isExternal(i, s));
+  private hasExternal(sources: Sources<FieldDefinition<any>>): boolean {
+    for (const [i, source] of sources.entries()) {
+      if (source && this.isExternal(i, source)) {
+        return true;
+      }
+    }
+    return false;
   }
 
   private isShareable(sourceIdx: number, field: FieldDefinition<any>): boolean {
@@ -1206,7 +1269,7 @@ class Merger {
    * Validates whether or not the use of the @override directive is correct.
    * return value is a list of fields that has been filtered to ignore overridden fields
    */
-  private validateOverride(sources: FieldOrUndefinedArray, dest: FieldDefinition<any>): FieldMergeContext {
+  private validateOverride(sources: Sources<FieldDefinition<any>>, dest: FieldDefinition<any>): FieldMergeContext {
     const result = new FieldMergeContext(sources);
 
     // For any field, we can't have more than one @override directive
@@ -1225,7 +1288,7 @@ class Merger {
     };
 
     // convert sources to a map so we don't have to keep scanning through the array to find a source
-    const { subgraphsWithOverride, subgraphMap } = sources.map((source, idx) => {
+    const mapped = mapSources(sources, (source, idx) => {
       if (!source) {
         // While the subgraph may not have the field directly, it could have "stand-in" for that field
         // through @interfaceObject, and it is those stand-ins that would be effectively overridden.
@@ -1248,7 +1311,11 @@ class Merger {
         isInterfaceObject: this.metadata(idx).isInterfaceObjectType(source.parent),
         overrideDirective: this.getOverrideDirective(idx, source),
       };
-    }).reduce((acc: ReduceResultType, elem) => {
+    });
+
+    const { subgraphsWithOverride, subgraphMap } = Array.from(
+      mapped.values()
+    ).reduce((acc: ReduceResultType, elem) => {
       if (elem !== undefined) {
         acc.subgraphMap[elem.name] = elem;
         if (elem.overrideDirective !== undefined) {
@@ -1323,13 +1390,13 @@ class Merger {
 
         // check to make sure that there is no conflicting @provides, @requires, or @external directives
         const fromIdx = this.names.indexOf(sourceSubgraphName);
-        const fromField = sources[fromIdx];
+        const fromField = sources.get(fromIdx);
         const { result: hasIncompatible, conflictingDirective, subgraph } = this.overrideConflictsWithOtherDirective({
           idx,
-          field: sources[idx],
+          field: sources.get(idx),
           subgraphName,
           fromIdx: this.names.indexOf(sourceSubgraphName),
-          fromField: sources[fromIdx],
+          fromField: sources.get(fromIdx),
         });
         if (hasIncompatible) {
           assert(conflictingDirective !== undefined, 'conflictingDirective should not be undefined');
@@ -1452,27 +1519,51 @@ class Merger {
     dest,
     mergeContext = new FieldMergeContext(sources),
   }: {
-    sources: FieldOrUndefinedArray,
+    sources: Sources<FieldDefinition<any>>,
     dest: FieldDefinition<any>,
     mergeContext: FieldMergeContext,
   }) {
-    if (sources.every((s, i) => s === undefined ? this.fieldsInSourceIfAbstractedByInterfaceObject(dest, i).every((f) => this.isExternal(i, f)) : this.isExternal(i, s))) {
-      const definingSubgraphs = sources.map((source, i) => {
+    let everySourceIsExternal = true;
+    for (const [i, s] of sources.entries()) {
+      if (s === undefined) {
+        everySourceIsExternal = everySourceIsExternal &&
+          this.fieldsInSourceIfAbstractedByInterfaceObject(dest, i)
+            .every((f) => this.isExternal(i, f));
+      } else {
+        everySourceIsExternal = everySourceIsExternal && this.isExternal(i, s);
+      }
+      if (!everySourceIsExternal) {
+        break;
+      }
+    }
+
+    if (everySourceIsExternal) {
+      const nodes: ASTNode[] = [];
+      const definingSubgraphs: string[] = [];
+      sources.forEach((source, i) => {
         if (source) {
-          return this.names[i];
+          definingSubgraphs.push(this.names[i]);
+          if (source.sourceAST) {
+            nodes.push(source.sourceAST);
+          }
+          return;
         }
 
         const itfObjectFields = this.fieldsInSourceIfAbstractedByInterfaceObject(dest, i);
         if (itfObjectFields.length === 0) {
-          return undefined;
+          return;
         }
-        return `${this.names[i]} (through @interaceObject ${printTypes(itfObjectFields.map((f) => f.parent))})`;
-      }).filter(isDefined);
-      const nodes = sources.map(source => source?.sourceAST).filter(s => s !== undefined) as ASTNode[];
+
+        definingSubgraphs.push(
+          `${this.names[i]} (through @interaceObject ${printTypes(itfObjectFields.map((f) => f.parent))})`
+        );
+      });
+
       this.errors.push(ERRORS.EXTERNAL_MISSING_ON_BASE.err(
         `Field "${dest.coordinate}" is marked @external on all the subgraphs in which it is listed (${printSubgraphNames(definingSubgraphs)}).`,
         { nodes }
       ));
+
       return;
     }
 
@@ -1485,7 +1576,7 @@ class Merger {
     this.recordAppliedDirectivesToMerge(withoutExternal, dest);
     this.addArgumentsShallow(withoutExternal, dest);
     for (const destArg of dest.arguments()) {
-      const subgraphArgs = withoutExternal.map(f => f?.argument(destArg.name));
+      const subgraphArgs = mapSources(withoutExternal, f => f?.argument(destArg.name));
       this.mergeArgument(subgraphArgs, destArg);
     }
     // Note that due to @interfaceObject, it's possible that `withoutExternal` is "empty" (has no
@@ -1518,9 +1609,10 @@ class Merger {
     //
     // Anyway, we still need to merge a type in the supergraph, so in that case
     // we use merge the external declarations directly.
-    const allTypesEqual = withoutExternal.every((s) => !s)
-      ? this.mergeTypeReference(sources, dest)
-      : this.mergeTypeReference(withoutExternal, dest);
+    const allTypesEqual = this.mergeTypeReference(
+      someSources(withoutExternal, isDefined) ? withoutExternal : sources,
+      dest,
+    );
 
     if (this.hasExternal(sources)) {
       this.validateExternalFields(sources, dest, allTypesEqual);
@@ -1529,7 +1621,7 @@ class Merger {
     this.addJoinDirectiveDirectives(sources, dest);
   }
 
-  private validateFieldSharing(sources: FieldOrUndefinedArray, dest: FieldDefinition<ObjectType>, mergeContext: FieldMergeContext) {
+  private validateFieldSharing(sources: Sources<FieldDefinition<ObjectType | InterfaceType>>, dest: FieldDefinition<ObjectType | InterfaceType>, mergeContext: FieldMergeContext) {
     const shareableSources: { subgraph: string, idx: number}[] = [];
     const nonShareableSources: { subgraph: string, idx: number}[] = [];
     const allResolving: { subgraph: string, field: FieldDefinition<any> }[] = [];
@@ -1593,7 +1685,7 @@ class Merger {
     }
   }
 
-  private validateExternalFields(sources: FieldOrUndefinedArray, dest: FieldDefinition<any>, allTypesEqual: boolean) {
+  private validateExternalFields(sources: Sources<FieldDefinition<any>>, dest: FieldDefinition<any>, allTypesEqual: boolean) {
     let hasInvalidTypes = false;
     const invalidArgsPresence = new Set<string>();
     const invalidArgsTypes = new Set<string>();
@@ -1641,7 +1733,7 @@ class Merger {
         code: ERRORS.EXTERNAL_ARGUMENT_MISSING,
         message: `Field "${dest.coordinate}" is missing argument "${destArg.coordinate}" in some subgraphs where it is marked @external: `,
         mismatchedElement: destArg,
-        subgraphElements: sources.map(s => s?.argument(destArg.name)),
+        subgraphElements: this.argumentSources(sources, destArg),
         mismatchAccessor: arg => arg ? `argument "${arg.coordinate}"` : undefined,
         supergraphElementPrinter: (elt, subgraphs) => `${elt} is declared in ${subgraphs}`,
         otherElementsPrinter: (_, subgraphs) => ` but not in ${subgraphs} (where "${dest.coordinate}" is @external).`,
@@ -1654,7 +1746,7 @@ class Merger {
         ERRORS.EXTERNAL_ARGUMENT_TYPE_MISMATCH,
         `Type of argument "${destArg.coordinate}" is incompatible across subgraphs (where "${dest.coordinate}" is marked @external): it has `,
         destArg,
-        sources.map(s => s?.argument(destArg.name)),
+        this.argumentSources(sources, destArg),
         arg => `type "${arg.type}"`
       );
     }
@@ -1664,19 +1756,27 @@ class Merger {
         ERRORS.EXTERNAL_ARGUMENT_DEFAULT_MISMATCH,
         `Argument "${destArg.coordinate}" has incompatible defaults across subgraphs (where "${dest.coordinate}" is marked @external): it has `,
         destArg,
-        sources.map(s => s?.argument(destArg.name)),
+        this.argumentSources(sources, destArg),
         arg => arg.defaultValue !== undefined ? `default value ${valueToString(arg.defaultValue, arg.type)}` : 'no default value'
       );
     }
   }
 
+  private argumentSources(sources: Sources<FieldDefinition<any>>, destArg: ArgumentDefinition<FieldDefinition<any>>) {
+    const argSources: Sources<ArgumentDefinition<FieldDefinition<any>>> = new Map;
+    for (const [i, source] of sources.entries()) {
+      argSources.set(i, source?.argument(destArg.name));
+    }
+    return argSources;
+  }
+
   private needsJoinField<T extends FieldDefinition<ObjectType | InterfaceType> | InputFieldDefinition>({
     sources,
     parentName,
     allTypesEqual,
     mergeContext,
   }: {
-    sources: (T | undefined)[],
+    sources: Sources<T>,
     parentName: string,
     allTypesEqual: boolean,
     mergeContext: FieldMergeContext,
@@ -1688,10 +1788,12 @@ class Merger {
     if (mergeContext.some(({ usedOverridden, overrideLabel }) => usedOverridden || !!overrideLabel)) {
       return true;
     }
-    
-    const coordinate = sources.find(isDefined)?.coordinate;
-    if (coordinate && this.fieldsWithFromContext.has(coordinate)) {
-      return true;
+
+    for (const source of sources.values()) {
+      const coordinate = source?.coordinate;
+      if (coordinate && this.fieldsWithFromContext.has(coordinate)) {
+        return true;
+      }
     }
 
     // We can avoid the join__field if:
@@ -1726,7 +1828,7 @@ class Merger {
       allTypesEqual,
       mergeContext,
     }: {
-      sources: (T | undefined)[],
+      sources: Sources<T>,
       dest: T,
       allTypesEqual: boolean,
       mergeContext: FieldMergeContext,
@@ -1804,14 +1906,14 @@ class Merger {
   // Returns `true` if the type references were all completely equal and `false` if some subtyping happened (or
   // if types were incompatible since an error is logged in this case but the method does not throw).
   private mergeTypeReference<TType extends Type, TElement extends NamedSchemaElementWithType<TType, any, any, any>>(
-    sources: (TElement | undefined)[],
+    sources: Sources<TElement>,
     dest: TElement,
     isInputPosition: boolean = false
   ): boolean {
     let destType: TType | undefined;
     let hasSubtypes = false;
     let hasIncompatible = false;
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -1849,9 +1951,15 @@ class Merger {
       const position = existing && existing.position !== thisPosition ? 'Both' : thisPosition;
       const examples = existing?.examples ?? {};
       if (!examples[thisPosition]) {
-        const idx = sources.findIndex((s) => !!s);
+        let idx = -1;
+        for (const [i, source] of sources.entries()) {
+          if (source) {
+            idx = i;
+            break;
+          }
+        }
         if (idx >= 0) {
-          const example = sources[idx]!;
+          const example = sources.get(idx)!;
           examples[thisPosition] = {
             coordinate: example.coordinate,
             sourceAST: example.sourceAST ? addSubgraphToASTNode(example.sourceAST, this.names[idx]) : undefined,
@@ -1903,9 +2011,9 @@ class Merger {
     );
   }
 
-  private addArgumentsShallow<T extends FieldDefinition<any> | DirectiveDefinition>(sources: (T | undefined)[], dest: T) {
+  private addArgumentsShallow<T extends FieldDefinition<any> | DirectiveDefinition>(sources: Sources<T>, dest: T) {
     const argNames = new Set<string>();
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -1923,63 +2031,71 @@ class Merger {
         const fromContextDirective = this.metadata(index).fromContextDirective();
         return fromContextDirective && isFederationDirectiveDefinedInSchema(fromContextDirective) && arg.appliedDirectivesOf(fromContextDirective).length >= 1;
       }
-      
-      const isContextualArray = sources.map((s, idx) => {
+
+      const isContextualMap = new Map<number, boolean>();
+      let sawContextualArgs = false;
+      sources.forEach((s, idx) => {
         const arg = s?.argument(argName);
-        return arg && isContextualArg(idx, arg);
+        const isContextual = arg && isContextualArg(idx, arg) || false;
+        isContextualMap.set(idx, isContextual);
+        if (isContextual) {
+          sawContextualArgs = true;
+        }
       });
-      
-      
 
-      if (isContextualArray.some((c) => c === true)) {
+      if (sawContextualArgs) {
         // if the argument is contextual in some subgraph, then it should also be contextual in other subgraphs,
         // unless it is nullable. Also, we need to remove it from the supergraph
-        isContextualArray.forEach((isContextual, idx) => {
-          const argument = sources[idx]?.argument(argName);
+        isContextualMap.forEach((isContextual, idx) => {
+          const argument = sources.get(idx)?.argument(argName);
           const argType = argument?.type;
           if (!isContextual && argument && argType && isNonNullType(argType) && argument.defaultValue === undefined) {
             this.errors.push(ERRORS.CONTEXTUAL_ARGUMENT_NOT_CONTEXTUAL_IN_ALL_SUBGRAPHS.err(
               `Argument "${arg.coordinate}" is contextual in at least one subgraph but in "${argument.coordinate}" it does not have @fromContext, is not nullable and has no default value.`,
-              { nodes: sourceASTs(sources[idx]?.argument(argName)) },
+              { nodes: sourceASTs(sources.get(idx)?.argument(argName)) },
             ));
           }
-          
+
           if (!isContextual && argument && argType && (isNullableType(argType) || argument.defaultValue !== undefined)) {
             // in this case, we want to issue a hint that the argument will not be present in the supergraph schema
             this.mismatchReporter.pushHint(new CompositionHint(
               HINTS.CONTEXTUAL_ARGUMENT_NOT_CONTEXTUAL_IN_ALL_SUBGRAPHS,
               `Contextual argument "${argument.coordinate}" will not be included in the supergraph since it is contextual in at least one subgraph`,
               undefined,
-            ));              
+            ));
           }
         });
-        
+
         arg.remove();
         continue;
       }
 
       // If all the sources that have the field have the argument, we do merge it
       // and we're good, but otherwise ...
-      if (sources.some((s) => s && !s.argument(argName))) {
+      if (someSources(sources, (s) => s && !s.argument(argName))) {
         // ... we don't merge the argument: some subgraphs wouldn't know what
         // to make of it and that would be dodgy at best. If the argument is
         // optional in all sources, then we can compose properly and just issue a
         // hint. But if it is mandatory, then we have to fail composition, otherwise
         // the query planner would have no choice but to generate invalidate queries.
-        const nonOptionalSources = sources.map((s, i) => s && s.argument(argName)?.isRequired() ? this.names[i] : undefined).filter((s) => !!s) as string[];
-        if (nonOptionalSources.length > 0) {
-          const nonOptionalSubgraphs = printSubgraphNames(nonOptionalSources);
-          const missingSources = printSubgraphNames(sources.map((s, i) => s && !s.argument(argName) ? this.names[i] : undefined).filter((s) => !!s) as string[]);
+        const nonOptionalSources = filterSources(
+          mapSources(sources, (s, i) => s && s.argument(argName)?.isRequired() ? this.names[i] : undefined),
+        );
+        if (nonOptionalSources.size > 0) {
+          const nonOptionalSubgraphs = printSubgraphNames(Array.from(nonOptionalSources.values()).filter(isDefined));
+          const missingSources = printSubgraphNames(Array.from(
+            mapSources(sources, (s, i) => s && !s.argument(argName) ? this.names[i] : undefined).values()
+          ).filter(isDefined));
           this.errors.push(ERRORS.REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH.err(
             `Argument "${arg.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`,
-            { nodes: sourceASTs(...sources.map((s) => s?.argument(argName))) },
+            { nodes: sourceASTs(...mapSources(sources, (s) => s?.argument(argName)).values()) },
           ));
         } else {
           this.mismatchReporter.reportMismatchHint({
             code: HINTS.INCONSISTENT_ARGUMENT_PRESENCE,
             message: `Optional argument "${arg.coordinate}" will not be included in the supergraph as it does not appear in all subgraphs: `,
             supergraphElement: arg,
-            subgraphElements: sources.map((s) => s ? s.argument(argName) : undefined),
+            subgraphElements: mapSources(sources, (s) => s ? s.argument(argName) : undefined),
             elementToString: _ => 'yes',
             supergraphElementPrinter: (_, subgraphs) => `it is defined in ${subgraphs}`,
             otherElementsPrinter: (_, subgraphs) => ` but not in ${subgraphs}`,
@@ -1992,19 +2108,19 @@ class Merger {
     }
   }
 
-  private mergeArgument(sources: (ArgumentDefinition<any> | undefined)[], dest: ArgumentDefinition<any>) {
+  private mergeArgument(sources: Sources<ArgumentDefinition<any>>, dest: ArgumentDefinition<any>) {
     this.mergeDescription(sources, dest);
     this.recordAppliedDirectivesToMerge(sources, dest);
     this.mergeTypeReference(sources, dest, true);
     this.mergeDefaultValue(sources, dest, 'Argument');
   }
 
-  private mergeDefaultValue<T extends ArgumentDefinition<any> | InputFieldDefinition>(sources: (T | undefined)[], dest: T, kind: string) {
+  private mergeDefaultValue<T extends ArgumentDefinition<any> | InputFieldDefinition>(sources: Sources<T>, dest: T, kind: string) {
     let destDefault;
     let hasSeenSource = false;
     let isInconsistent = false;
     let isIncompatible = false;
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -2063,7 +2179,7 @@ class Merger {
     }
   }
 
-  private mergeInterface(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType) {
+  private mergeInterface(sources: Sources<InterfaceType | ObjectType>, dest: InterfaceType) {
     const hasKey = this.validateInterfaceKeys(sources, dest);
     this.validateInterfaceObjects(sources, dest);
 
@@ -2072,7 +2188,7 @@ class Merger {
       if (!hasKey) {
         this.hintOnInconsistentValueTypeField(sources, dest, destField);
       }
-      const subgraphFields = sources.map(t => t?.field(destField.name));
+      const subgraphFields = mapSources(sources, t => t?.field(destField.name));
       const mergeContext = this.validateOverride(subgraphFields, destField);
       this.mergeField({
         sources: subgraphFields,
@@ -2083,7 +2199,7 @@ class Merger {
   }
 
   // Returns whether the interface has a key (even a non-resolvable one) in any subgraph.
-  private validateInterfaceKeys(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType): boolean {
+  private validateInterfaceKeys(sources: Sources<InterfaceType | ObjectType>, dest: InterfaceType): boolean {
     // Remark: it might be ok to filter @inaccessible types in `supergraphImplementations`, but this requires
     // some more thinking (and I'm not even sure it makes a practical difference given the rules for validity
     // of @inaccessible) and it will be backward compatible to filter them later, while the reverse wouldn't
@@ -2121,7 +2237,7 @@ class Merger {
     return hasKey;
   }
 
-  private validateInterfaceObjects(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType) {
+  private validateInterfaceObjects(sources: Sources<InterfaceType | ObjectType>, dest: InterfaceType) {
     const supergraphImplementations = dest.possibleRuntimeTypes();
 
     // Validates that if a source defines the interface as an @interfaceObject, then it doesn't define any
@@ -2150,8 +2266,8 @@ class Merger {
     }
   }
 
-  private mergeUnion(sources: (UnionType | undefined)[], dest: UnionType) {
-    for (const source of sources) {
+  private mergeUnion(sources: Sources<UnionType>, dest: UnionType) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -2167,7 +2283,7 @@ class Merger {
     }
   }
 
-  private addJoinUnionMember(sources: (UnionType | undefined)[], dest: UnionType, member: ObjectType) {
+  private addJoinUnionMember(sources: Sources<UnionType>, dest: UnionType, member: ObjectType) {
     const joinUnionMemberDirective = this.joinSpec.unionMemberDirective(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 ...
@@ -2189,11 +2305,11 @@ class Merger {
   }
 
   private hintOnInconsistentUnionMember(
-    sources: (UnionType | undefined)[],
+    sources: Sources<UnionType>,
     dest: UnionType,
     memberName: string
   ) {
-    for (const source of sources) {
+    for (const source of sources.values()) {
       // As soon as we find a subgraph that has the type but not the member, we hint.
       if (source && !source.hasTypeMember(memberName)) {
         this.mismatchReporter.reportMismatchHint({
@@ -2210,7 +2326,7 @@ class Merger {
     }
   }
 
-  private mergeEnum(sources: (EnumType | undefined)[], dest: EnumType) {
+  private mergeEnum(sources: Sources<EnumType>, dest: EnumType) {
     let usage = this.enumUsages.get(dest.name);
     if (!usage) {
       // If the enum is unused, we have a choice to make. We could skip the enum entirely (after all, exposing an unreferenced type mostly "pollutes" the supergraph API), but
@@ -2226,7 +2342,7 @@ class Merger {
       ));
     }
 
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         continue;
       }
@@ -2238,6 +2354,7 @@ class Merger {
         }
       }
     }
+
     for (const value of dest.values) {
       this.mergeEnumValue(sources, dest, value, usage);
     }
@@ -2246,13 +2363,13 @@ class Merger {
     if (dest.values.length === 0) {
       this.errors.push(ERRORS.EMPTY_MERGED_ENUM_TYPE.err(
         `None of the values of enum type "${dest}" are defined consistently in all the subgraphs defining that type. As only values common to all subgraphs are merged, this would result in an empty type.`,
-        { nodes: sourceASTs(...sources) },
+        { nodes: sourceASTs(...sources.values()) },
       ));
     }
   }
 
   private mergeEnumValue(
-    sources: (EnumType | undefined)[],
+    sources: Sources<EnumType>,
     dest: EnumType,
     value: EnumValue,
     { position, examples }: EnumTypeUsage,
@@ -2261,7 +2378,7 @@ class Merger {
     // but we do so because:
     // 1. this will catch any problems merging the description/directives (which feels like a good thing).
     // 2. it easier to see if the value is marked @inaccessible.
-    const valueSources = sources.map(s => s?.value(value.name));
+    const valueSources = mapSources(sources, s => s?.value(value.name));
     this.mergeDescription(valueSources, value);
     this.recordAppliedDirectivesToMerge(valueSources, value);
     this.addJoinEnumValue(valueSources, value);
@@ -2275,7 +2392,11 @@ class Merger {
     // So in particular, the value will be in the supergraph only if it is either an "output only" enum, or if the value is in all subgraphs.
     // Note that (like for input object fields), manually marking the value as @inaccessible let's use skips any check and add the value
     // regardless of inconsistencies.
-    if (!isInaccessible && position !== 'Output' && sources.some((source) => source && !source.value(value.name))) {
+    if (
+      !isInaccessible &&
+      position !== 'Output' &&
+      someSources(sources, (source) => source && !source.value(value.name))
+    ) {
       // We have a source (subgraph) that _has_ the enum type but not that particular enum value. If we've in the "both input and output usages",
       // that's where we have to fail. But if we're in the "only input" case, we simply don't merge that particular value and hint about it.
       if (position === 'Both') {
@@ -2315,7 +2436,7 @@ class Merger {
     }
   }
 
-  private addJoinEnumValue(sources: (EnumValue | undefined)[], dest: EnumValue) {
+  private addJoinEnumValue(sources: Sources<EnumValue>, dest: EnumValue) {
     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 ...
@@ -2336,12 +2457,12 @@ class Merger {
   }
 
   private hintOnInconsistentOutputEnumValue(
-    sources: (EnumType | undefined)[],
+    sources: Sources<EnumType>,
     dest: EnumType,
     value: EnumValue,
   ) {
     const valueName: string = value.name
-    for (const source of sources) {
+    for (const source of sources.values()) {
       // As soon as we find a subgraph that has the type but not the member, we hint.
       if (source && !source.value(valueName)) {
         this.mismatchReporter.reportMismatchHint({
@@ -2359,7 +2480,7 @@ class Merger {
     }
   }
 
-  private mergeInput(sources: (InputObjectType | undefined)[], dest: InputObjectType) {
+  private mergeInput(sources: Sources<InputObjectType>, dest: InputObjectType) {
     const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name);
 
     // Like for other inputs, we add all the fields found in any subgraphs initially as a simple mean to have a complete list of
@@ -2370,27 +2491,31 @@ class Merger {
       // We merge the details of the field first, even if we may remove it afterwards because 1) this ensure we always checks type
       // compatibility between definitions and 2) we actually want to see if the result is marked inaccessible or not and it makes
       // that easier.
-      this.mergeInputField(sources.map(t => t?.field(name)), destField);
+      this.mergeInputField(mapSources(sources, t => t?.field(name)), destField);
       const isInaccessible = inaccessibleInSupergraph && destField.hasAppliedDirective(inaccessibleInSupergraph.definition);
       // Note that if the field is manually marked @inaccessible, we can always accept it to be inconsistent between subgraphs since
       // it won't be exposed in the API, and we don't hint about it because we're just doing what the user is explicitely asking.
-      if (!isInaccessible && sources.some((source) => source && !source.field(name))) {
+      if (!isInaccessible && someSources(sources, (source) => source && !source.field(name))) {
         // One of the subgraph has the input type but not that field. If the field is optional, we remove it for the supergraph
         // and issue a hint. But if it is required, we have to error out.
-        const nonOptionalSources = sources.map((s, i) => s && s.field(name)?.isRequired() ? this.names[i] : undefined).filter((s) => !!s) as string[];
-        if (nonOptionalSources.length > 0) {
-          const nonOptionalSubgraphs = printSubgraphNames(nonOptionalSources);
-          const missingSources = printSubgraphNames(sources.map((s, i) => s && !s.field(name) ? this.names[i] : undefined).filter((s) => !!s) as string[]);
+        const nonOptionalSources = filterSources(
+          mapSources(sources, (s, i) => s && s.field(name)?.isRequired() ? this.names[i] : undefined),
+        );
+        if (nonOptionalSources.size > 0) {
+          const nonOptionalSubgraphs = printSubgraphNames(Array.from(nonOptionalSources.values()).filter(isDefined));
+          const missingSources = printSubgraphNames(Array.from(
+            mapSources(sources, (s, i) => s && !s.field(name) ? this.names[i] : undefined).values()
+          ).filter(isDefined));
           this.errors.push(ERRORS.REQUIRED_INPUT_FIELD_MISSING_IN_SOME_SUBGRAPH.err(
             `Input object field "${destField.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`,
-            { nodes: sourceASTs(...sources.map((s) => s?.field(name))) },
+            { nodes: sourceASTs(...mapSources(sources, (s) => s?.field(name)).values()) },
           ));
         } else {
           this.mismatchReporter.reportMismatchHint({
             code: HINTS.INCONSISTENT_INPUT_OBJECT_FIELD,
             message: `Input object field "${destField.name}" will not be added to "${dest}" in the supergraph as it does not appear in all subgraphs: `,
             supergraphElement: destField,
-            subgraphElements: sources.map((s) => s ? s.field(name) : undefined),
+            subgraphElements: mapSources(sources, (s) => s ? s.field(name) : undefined),
             elementToString: _ => 'yes',
             // Note that the first callback is for element that are "like the supergraph" and we've pass `destField` which we havne't yet removed.
             supergraphElementPrinter: (_, subgraphs) => `it is defined in ${subgraphs}`,
@@ -2407,12 +2532,12 @@ class Merger {
     if (!dest.hasFields()) {
       this.errors.push(ERRORS.EMPTY_MERGED_INPUT_TYPE.err(
         `None of the fields of input object type "${dest}" are consistently defined in all the subgraphs defining that type. As only fields common to all subgraphs are merged, this would result in an empty type.`,
-        { nodes: sourceASTs(...sources) },
+        { nodes: sourceASTs(...sources.values()) },
       ));
     }
   }
 
-  private mergeInputField(sources: (InputFieldDefinition | undefined)[], dest: InputFieldDefinition) {
+  private mergeInputField(sources: Sources<InputFieldDefinition>, dest: InputFieldDefinition) {
     this.mergeDescription(sources, dest);
     this.recordAppliedDirectivesToMerge(sources, dest);
     const allTypesEqual = this.mergeTypeReference(sources, dest, true);
@@ -2421,7 +2546,7 @@ class Merger {
     this.mergeDefaultValue(sources, dest, 'Input field');
   }
 
-  private mergeDirectiveDefinition(sources: (DirectiveDefinition | undefined)[], dest: DirectiveDefinition) {
+  private mergeDirectiveDefinition(sources: Sources<DirectiveDefinition>, dest: DirectiveDefinition) {
     // We have 2 behavior depending on the kind of directives:
     // 1) for the few handpicked type system directives that we merge, we always want to keep
     //   them (it's ok if a subgraph decided to not include the definition because that particular
@@ -2437,7 +2562,7 @@ class Merger {
     //   locations, we only expose locations that are common everywhere).
     if (this.composeDirectiveManager.directiveExistsInSupergraph(dest.name)) {
       this.mergeCustomCoreDirective(dest);
-    } else if (sources.some((s, idx) => s && this.isMergedDirective(this.names[idx], s))) {
+    } else if (someSources(sources, (s, idx) => s && this.isMergedDirective(this.names[idx], s))) {
       this.mergeExecutableDirectiveDefinition(sources, dest);
     }
   }
@@ -2517,21 +2642,21 @@ class Merger {
       dest.repeatable = def.repeatable;
       dest.description = def.description;
       dest.addLocations(...def.locations);
-      this.addArgumentsShallow([def], dest);
+      this.addArgumentsShallow(sourcesFromArray([def]), dest);
       for (const arg of def.arguments()) {
         const destArg = dest.argument(arg.name);
         assert(destArg, 'argument must exist on destination directive');
-        this.mergeArgument([arg], destArg);
+        this.mergeArgument(sourcesFromArray([arg]), destArg);
       }
     }
   }
 
-  private mergeExecutableDirectiveDefinition(sources: (DirectiveDefinition | undefined)[], dest: DirectiveDefinition) {
+  private mergeExecutableDirectiveDefinition(sources: Sources<DirectiveDefinition>, dest: DirectiveDefinition) {
     let repeatable: boolean | undefined = undefined;
     let inconsistentRepeatable = false;
     let locations: DirectiveLocation[] | undefined = undefined;
     let inconsistentLocations = false;
-    for (const source of sources) {
+    for (const source of sources.values()) {
       if (!source) {
         // An executable directive could appear in any place of a query and thus get to any subgraph, so we cannot keep an
         // executable directive unless it is in all subgraphs. We use an 'intersection' strategy.
@@ -2617,7 +2742,7 @@ class Merger {
     // Doing args last, mostly so we don't bother adding if the directive doesn't make it in.
     this.addArgumentsShallow(sources, dest);
     for (const destArg of dest.arguments()) {
-      const subgraphArgs = sources.map(f => f?.argument(destArg.name));
+      const subgraphArgs = mapSources(sources, f => f?.argument(destArg.name));
       this.mergeArgument(subgraphArgs, destArg);
     }
   }
@@ -2634,7 +2759,7 @@ class Merger {
   // In general, we want to merge applied directives after merging elements, the one exception
   // is @inaccessible, which is necessary to exist in the supergraph for EnumValues to properly
   // determine whether the fact that a value is both input / output will matter
-  private recordAppliedDirectivesToMerge(sources: (SchemaElement<any, any> | undefined)[], dest: SchemaElement<any, any>) {
+  private recordAppliedDirectivesToMerge(sources: Sources<SchemaElement<any, any>>, dest: SchemaElement<any, any>) {
     const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name);
     const inaccessibleName = inaccessibleInSupergraph?.definition.name;
     const names = this.gatherAppliedDirectiveNames(sources);
@@ -2668,7 +2793,7 @@ class Merger {
     this.appliedDirectivesToMerge = [];
   }
 
-  private gatherAppliedDirectiveNames(sources: (SchemaElement<any, any> | undefined)[]): Set<string> {
+  private gatherAppliedDirectiveNames(sources: Sources<SchemaElement<any, any>>): Set<string> {
     const names = new Set<string>();
     sources.forEach((source, idx) => {
       if (source) {
@@ -2682,7 +2807,7 @@ class Merger {
     return names;
   }
 
-  private mergeAppliedDirective(name: string, sources: (SchemaElement<any, any> | undefined)[], dest: SchemaElement<any, any>) {
+  private mergeAppliedDirective(name: string, sources: Sources<SchemaElement<any, any>>, dest: SchemaElement<any, any>) {
     // TODO: we currently "only" merge together applications that have the exact same arguments (with defaults expanded however),
     // but when an argument is an input object type, we should (?) ignore those fields that will not be included in the supergraph
     // due the intersection merging of input types, otherwise the merged value may be invalid for the supergraph.
@@ -2788,7 +2913,7 @@ class Merger {
       && valueEquals(application1.arguments(true), application2.arguments(true));
   }
 
-  private mergeSchemaDefinition(sources: SchemaDefinition[], dest: SchemaDefinition) {
+  private mergeSchemaDefinition(sources: Sources<SchemaDefinition>, dest: SchemaDefinition) {
     this.mergeDescription(sources, dest);
     this.recordAppliedDirectivesToMerge(sources, dest);
     // Before merging, we actually rename all the root types to their default name
@@ -2800,10 +2925,10 @@ class Merger {
     for (const rootKind of allSchemaRootKinds) {
       let rootType: string | undefined;
       let isIncompatible = false;
-      for (const sourceType of sources.map(s => filteredRoot(s, rootKind))) {
-        if (!sourceType) {
-          continue;
-        }
+      for (const source of sources.values()) {
+        if (!source) continue;
+        const sourceType = filteredRoot(source, rootKind);
+        if (!sourceType) continue;
         if (rootType) {
           isIncompatible = isIncompatible || rootType !== sourceType.name;
         } else {
@@ -2854,7 +2979,7 @@ class Merger {
   // subgraph directive applications to be reflected (unapplied) in the
   // supergraph, using the @join__directive(graphs,name,args) directive.
   private addJoinDirectiveDirectives(
-    sources: (SchemaElement<any, any> | undefined)[],
+    sources: Sources<SchemaElement<any, any>>,
     dest: SchemaElement<any, any>,
   ) {
     const joinsByDirectiveName: {
@@ -3220,9 +3345,15 @@ class Merger {
     });
   }
 
-  private validateSubscriptionField(sources: FieldOrUndefinedArray) {
+  private validateSubscriptionField(sources: Sources<FieldDefinition<any>>) {
     // no subgraph marks field as @shareable
-    const fieldsWithShareable = sources.filter((src, idx) => src && src.appliedDirectivesOf(this.metadata(idx).shareableDirective()).length > 0);
+
+    const fieldsWithShareable: FieldDefinition<any>[] = [];
+    for (const [idx, source] of sources.entries()) {
+      if (source && source.hasAppliedDirective(this.metadata(idx).shareableDirective())) {
+        fieldsWithShareable.push(source);
+      }
+    }
     if (fieldsWithShareable.length > 0) {
       const nodes = sourceASTs(...fieldsWithShareable);
       this.errors.push(ERRORS.INVALID_FIELD_SHARING.err(
diff --git a/composition-js/src/merging/reporter.ts b/composition-js/src/merging/reporter.ts
index 4eb64a889..42d1075d9 100644
--- a/composition-js/src/merging/reporter.ts
+++ b/composition-js/src/merging/reporter.ts
@@ -1,6 +1,7 @@
 import { addSubgraphToASTNode, assert, ErrorCodeDefinition, joinStrings, MultiMap, NamedSchemaElement, printSubgraphNames, SubgraphASTNode } from '@apollo/federation-internals';
 import { ASTNode, GraphQLError } from 'graphql';
 import { CompositionHint, HintCodeDefinition } from '../hints';
+import { Sources } from './merge';
 
 export class MismatchReporter {
   pushError: (error: GraphQLError) => void;
@@ -15,7 +16,7 @@ export class MismatchReporter {
     code: ErrorCodeDefinition,
     message: string,
     mismatchedElement:TMismatched,
-    subgraphElements: (TMismatched | undefined)[],
+    subgraphElements: Sources<TMismatched>,
     mismatchAccessor: (elt: TMismatched, isSupergraph: boolean) => string | undefined
   ) {
     this.reportMismatch(
@@ -37,7 +38,7 @@ export class MismatchReporter {
   reportMismatchErrorWithoutSupergraph<TMismatched extends { sourceAST?: ASTNode }>(
     code: ErrorCodeDefinition,
     message: string,
-    subgraphElements: (TMismatched | undefined)[],
+    subgraphElements: Sources<TMismatched>,
     mismatchAccessor: (elt: TMismatched, isSupergraph: boolean) => string | undefined
   ) {
     this.reportMismatch(
@@ -71,7 +72,7 @@ export class MismatchReporter {
     code: ErrorCodeDefinition,
     message: string,
     mismatchedElement: TMismatched,
-    subgraphElements: (TMismatched | undefined)[],
+    subgraphElements: Sources<TMismatched>,
     mismatchAccessor: (elt: TMismatched | undefined, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
     otherElementsPrinter: (elt: string, subgraphs: string) => string,
@@ -112,7 +113,7 @@ export class MismatchReporter {
     code: HintCodeDefinition,
     message: string,
     supergraphElement: TMismatched,
-    subgraphElements: (TMismatched | undefined)[],
+    subgraphElements: Sources<TMismatched>,
     targetedElement?: NamedSchemaElement<any, any, any>
     elementToString: (elt: TMismatched, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
@@ -143,7 +144,7 @@ export class MismatchReporter {
   // Not meant to be used directly: use `reportMismatchError` or `reportMismatchHint` instead.
   private reportMismatch<TMismatched extends { sourceAST?: ASTNode }>(
     supergraphElement:TMismatched | undefined,
-    subgraphElements: (TMismatched | undefined)[],
+    subgraphElements: Sources<TMismatched>,
     mismatchAccessor: (element: TMismatched, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
     otherElementsPrinter: (elt: string, subgraphs: string) => string,

From 4b041180f477171aada9822f9968df53356bfd3e Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@benjamn.com>
Date: Tue, 2 Jul 2024 13:05:31 -0400
Subject: [PATCH 3/5] Do more work in addFieldsShallow method.

Since we already loop over all input sources to add shallow versions of
fields to the dest object in addFieldsShallow, we can return a
representation of what was added to be used later in mergeObject,
mergeInterface, and mergeInput, yielding a speedup.

The source of the speedup is the use of sparse Sources maps, so not all
subgraphs need to have an entry in Sources, though some subgraphs still
do have (intentionally) undefined entries, indicating the subgraph does
not contribute a particular field, but might matter for validation of
the field.

I'm aware JavaScript also supports "sparse arrays" which are Array
objects with "holes" in them (missing elements, not just undefined), but
not all operations (such as sparseArray.entries()) skip the holes, so it
seemed better/safer to use an explicitly sparse data structure like Map.
---
 composition-js/src/merging/merge.ts | 118 ++++++++++++++++++++--------
 1 file changed, 86 insertions(+), 32 deletions(-)

diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts
index 8d9baaf16..573aaf36d 100644
--- a/composition-js/src/merging/merge.ts
+++ b/composition-js/src/merging/merge.ts
@@ -975,18 +975,17 @@ class Merger {
     const isValueType = !isEntity && !dest.isRootType();
     const isSubscription = dest.isSubscriptionRootType();
 
-    this.addFieldsShallow(sources, dest);
-    if (!dest.hasFields()) {
+    const added = this.addFieldsShallow(sources, dest);
+    if (!added.size) {
       // This can happen for a type that existing in the subgraphs but had only non-merged fields
       // (currently, this can only be the 'Query' type, in the rare case where the federated schema
       // exposes no queries) .
       dest.remove();
     } else {
-      for (const destField of dest.fields()) {
+      added.forEach((subgraphFields, destField) => {
         if (isValueType) {
           this.hintOnInconsistentValueTypeField(sources, dest, destField);
         }
-        const subgraphFields = mapSources(sources, t => t?.field(destField.name));
         const mergeContext = this.validateOverride(subgraphFields, destField);
 
         if (isSubscription) {
@@ -999,7 +998,7 @@ class Merger {
           mergeContext,
         });
         this.validateFieldSharing(subgraphFields, destField, mergeContext);
-      }
+      });
     }
   }
 
@@ -1149,20 +1148,77 @@ class Merger {
     });
   }
 
-  private addFieldsShallow<T extends ObjectType | InterfaceType | InputObjectType>(sources: Sources<T>, dest: T) {
-    for (const source of sources.values()) {
-      if (!source) {
-        continue;
-      }
-      for (const field of source.fields()) {
-        if (!isMergedField(field)) {
-          continue;
+  private addFieldsShallow<T extends ObjectType | InterfaceType | InputObjectType>(
+    sources: Sources<T>,
+    dest: T,
+  ) {
+    type FieldDef = FieldDefinition<ObjectType | InterfaceType> | InputFieldDefinition;
+    const added = new Map<FieldDef, Sources<FieldDef>>();
+    const fieldsToAdd = new Map<number, Set<FieldDef | undefined>>();
+    function fieldSet(sourceIndex: number): Set<FieldDef | undefined> {
+      let set = fieldsToAdd.get(sourceIndex);
+      if (!set) fieldsToAdd.set(sourceIndex, set = new Set);
+      return set;
+    }
+
+    const extraSources: Sources<FieldDef> = new Map;
+
+    sources.forEach((source, sourceIndex) => {
+      const schema = this.subgraphsSchema[sourceIndex];
+      const fields = fieldSet(sourceIndex);
+
+      // If a source is undefined, it may still have an @interfaceObject object
+      // for one of the interfaces implemented by the object in question.
+      if (isObjectType(dest) || isInterfaceType(dest)) {
+        for (const itf of dest.interfaces()) {
+          const itfType = schema.type(itf.name);
+          const subgraph = this.subgraphs.get(this.names[sourceIndex]);
+          if (
+            itfType &&
+            isObjectType(itfType) &&
+            subgraph?.metadata().isInterfaceObjectType(itfType)
+          ) {
+            // This marks the subgraph as having a relevant @interfaceObject,
+            // even though we do not actively add the itfType.fields().
+            extraSources.set(sourceIndex, undefined);
+          }
         }
-        if (!dest.field(field.name)) {
-          dest.addField(field.name);
+      }
+
+      if (source) {
+        for (const field of source.fields()) {
+          fields.add(field);
         }
       }
-    }
+
+      if (schema.type(dest.name)) {
+        // Our needsJoinField logic adds @join__field if any subgraphs define
+        // the parent type containing the field but not the field itself. In
+        // those cases, for each field we add, we need to add undefined entries
+        // for each subgraph that defines the parent object/interface/input
+        // type. We do this by populating parentTypeOnlySources with undefined
+        // entries here, and then creating each new destSources map from that
+        // starting set.
+        extraSources.set(sourceIndex, undefined);
+      }
+    });
+
+    fieldsToAdd.forEach((fieldSet, sourceIndex) => {
+      fieldSet.forEach(field => {
+        if (field && isMergedField(field)) {
+          const destField = dest.field(field.name) || dest.addField(field.name);
+          let sources = added.get(destField)!;
+          if (!sources) {
+            sources = new Map(extraSources);
+            added.set(destField, sources);
+          }
+          sources.set(sourceIndex, field);
+        }
+      });
+    });
+
+    type FieldDefExact = T extends ObjectType | InterfaceType ? FieldDefinition<T> : InputFieldDefinition;
+    return added as Map<FieldDefExact, Sources<FieldDefExact>>;
   }
 
   private isExternal(sourceIdx: number, field: FieldDefinition<any> | InputFieldDefinition) {
@@ -2183,19 +2239,18 @@ class Merger {
     const hasKey = this.validateInterfaceKeys(sources, dest);
     this.validateInterfaceObjects(sources, dest);
 
-    this.addFieldsShallow(sources, dest);
-    for (const destField of dest.fields()) {
+    const added = this.addFieldsShallow(sources, dest);
+    added.forEach((subgraphFields, destField) => {
       if (!hasKey) {
         this.hintOnInconsistentValueTypeField(sources, dest, destField);
       }
-      const subgraphFields = mapSources(sources, t => t?.field(destField.name));
       const mergeContext = this.validateOverride(subgraphFields, destField);
       this.mergeField({
         sources: subgraphFields,
         dest: destField,
         mergeContext,
       });
-    }
+    });
   }
 
   // Returns whether the interface has a key (even a non-resolvable one) in any subgraph.
@@ -2480,42 +2535,41 @@ class Merger {
     }
   }
 
-  private mergeInput(sources: Sources<InputObjectType>, dest: InputObjectType) {
+  private mergeInput(inputSources: Sources<InputObjectType>, dest: InputObjectType) {
     const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name);
 
     // Like for other inputs, we add all the fields found in any subgraphs initially as a simple mean to have a complete list of
     // field to iterate over, but we will remove those that are not in all subgraphs.
-    this.addFieldsShallow(sources, dest);
-    for (const destField of dest.fields()) {
-      const name = destField.name
+    const added = this.addFieldsShallow(inputSources, dest);
+    added.forEach((subgraphFields, destField) => {
       // We merge the details of the field first, even if we may remove it afterwards because 1) this ensure we always checks type
       // compatibility between definitions and 2) we actually want to see if the result is marked inaccessible or not and it makes
       // that easier.
-      this.mergeInputField(mapSources(sources, t => t?.field(name)), destField);
+      this.mergeInputField(subgraphFields, destField);
       const isInaccessible = inaccessibleInSupergraph && destField.hasAppliedDirective(inaccessibleInSupergraph.definition);
       // Note that if the field is manually marked @inaccessible, we can always accept it to be inconsistent between subgraphs since
       // it won't be exposed in the API, and we don't hint about it because we're just doing what the user is explicitely asking.
-      if (!isInaccessible && someSources(sources, (source) => source && !source.field(name))) {
+      if (!isInaccessible && someSources(subgraphFields, field => !field)) {
         // One of the subgraph has the input type but not that field. If the field is optional, we remove it for the supergraph
         // and issue a hint. But if it is required, we have to error out.
         const nonOptionalSources = filterSources(
-          mapSources(sources, (s, i) => s && s.field(name)?.isRequired() ? this.names[i] : undefined),
+          mapSources(subgraphFields, (field, i) => field?.isRequired() ? this.names[i] : undefined),
         );
         if (nonOptionalSources.size > 0) {
           const nonOptionalSubgraphs = printSubgraphNames(Array.from(nonOptionalSources.values()).filter(isDefined));
           const missingSources = printSubgraphNames(Array.from(
-            mapSources(sources, (s, i) => s && !s.field(name) ? this.names[i] : undefined).values()
+            mapSources(subgraphFields, (field, i) => !field ? this.names[i] : undefined).values()
           ).filter(isDefined));
           this.errors.push(ERRORS.REQUIRED_INPUT_FIELD_MISSING_IN_SOME_SUBGRAPH.err(
             `Input object field "${destField.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`,
-            { nodes: sourceASTs(...mapSources(sources, (s) => s?.field(name)).values()) },
+            { nodes: sourceASTs(...subgraphFields.values()) },
           ));
         } else {
           this.mismatchReporter.reportMismatchHint({
             code: HINTS.INCONSISTENT_INPUT_OBJECT_FIELD,
             message: `Input object field "${destField.name}" will not be added to "${dest}" in the supergraph as it does not appear in all subgraphs: `,
             supergraphElement: destField,
-            subgraphElements: mapSources(sources, (s) => s ? s.field(name) : undefined),
+            subgraphElements: subgraphFields,
             elementToString: _ => 'yes',
             // Note that the first callback is for element that are "like the supergraph" and we've pass `destField` which we havne't yet removed.
             supergraphElementPrinter: (_, subgraphs) => `it is defined in ${subgraphs}`,
@@ -2526,13 +2580,13 @@ class Merger {
         // Note that we remove the element after the hint/error because we access the parent in the hint message.
         destField.remove();
       }
-    }
+    });
 
     // We could be left with an input type with no fields, and that's invalid in graphQL
     if (!dest.hasFields()) {
       this.errors.push(ERRORS.EMPTY_MERGED_INPUT_TYPE.err(
         `None of the fields of input object type "${dest}" are consistently defined in all the subgraphs defining that type. As only fields common to all subgraphs are merged, this would result in an empty type.`,
-        { nodes: sourceASTs(...sources.values()) },
+        { nodes: sourceASTs(...inputSources.values()) },
       ));
     }
   }

From eca8ce6adbe5ff7c8935e7d8b262e36263a55e0d Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@benjamn.com>
Date: Mon, 8 Jul 2024 09:38:09 -0400
Subject: [PATCH 4/5] More comments about Sources<T> handling.

---
 composition-js/src/merging/merge.ts | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts
index 573aaf36d..9e6595dc5 100644
--- a/composition-js/src/merging/merge.ts
+++ b/composition-js/src/merging/merge.ts
@@ -93,8 +93,15 @@ import { inspect } from "util";
 import { collectCoreDirectivesToCompose, CoreDirectiveInSubgraphs } from "./coreDirectiveCollector";
 import { CompositionOptions } from "../compose";
 
+// A Sources<T> map represents the contributions from each subgraph of the given
+// element type T. The numeric keys correspond to the indexes of the subgraphs
+// in the original Subgraphs/names/subgraphsSchema arrays. When merging a
+// specific type or field, this Map will ideally contain far fewer entries than
+// the total number of subgraphs, though it will sometimes need to contain
+// explicitly undefined entries (hence T | undefined).
 export type Sources<T> = Map<number, T | undefined>;
 
+// Like Array.prototype.map, but for Sources<T> maps.
 function mapSources<T, R>(
   sources: Sources<T>,
   mapper: (source: T | undefined, index: number) => R,
@@ -106,6 +113,9 @@ function mapSources<T, R>(
   return result;
 }
 
+// Removes all undefined sources from a given Sources<T> map. In other words,
+// this is not the same as Array.prototype.filter, which takes an arbitrary
+// boolean predicate.
 function filterSources<T>(sources: Sources<T>): Sources<T> {
   const result: Sources<T> = new Map;
   sources.forEach((source, idx) => {
@@ -116,6 +126,7 @@ function filterSources<T>(sources: Sources<T>): Sources<T> {
   return result;
 }
 
+// Like Array.prototype.some, but for Sources<T> maps.
 function someSources<T>(sources: Sources<T>, predicate: (source: T | undefined, index: number) => boolean | undefined): boolean {
   for (const [idx, source] of sources.entries()) {
     if (predicate(source, idx)) {
@@ -125,6 +136,7 @@ function someSources<T>(sources: Sources<T>, predicate: (source: T | undefined,
   return false;
 }
 
+// Converts an array of T | undefined into a dense Sources<T> map.
 export function sourcesFromArray<T>(array: (T | undefined)[]): Sources<T> {
   const sources: Sources<T> = new Map;
   array.forEach((source, idx) => {
@@ -1196,9 +1208,9 @@ class Merger {
         // the parent type containing the field but not the field itself. In
         // those cases, for each field we add, we need to add undefined entries
         // for each subgraph that defines the parent object/interface/input
-        // type. We do this by populating parentTypeOnlySources with undefined
-        // entries here, and then creating each new destSources map from that
-        // starting set.
+        // type. We do this by populating extraSources with undefined entries
+        // here, then create each new Sources map from that starting set (see
+        // `new Map(extraSources)` below).
         extraSources.set(sourceIndex, undefined);
       }
     });
@@ -1217,7 +1229,14 @@ class Merger {
       });
     });
 
-    type FieldDefExact = T extends ObjectType | InterfaceType ? FieldDefinition<T> : InputFieldDefinition;
+    // Although Map<FieldDef, Sources<FieldDef>> makes the work of this method
+    // easier, we return a more specific type that depends conditionally on T,
+    // so (for example) callers receive a Map<FieldDefinition<ObjectType>,
+    // Sources<FieldDefinition<ObjectType>> when T extends ObjectType, rather
+    // than the more generic Map<FieldDef, Source<FieldDef>>.
+    type FieldDefExact = T extends ObjectType | InterfaceType
+      ? FieldDefinition<T>
+      : T extends InputObjectType ? InputFieldDefinition : never;
     return added as Map<FieldDefExact, Sources<FieldDefExact>>;
   }
 

From f5f6a799d6b3675eecb0eaec7a816d746cd136b2 Mon Sep 17 00:00:00 2001
From: "Sachin D. Shinde" <sachin@apollographql.com>
Date: Mon, 8 Jul 2024 19:46:26 -0400
Subject: [PATCH 5/5] Add changeset entry

---
 .changeset/smooth-melons-study.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/smooth-melons-study.md

diff --git a/.changeset/smooth-melons-study.md b/.changeset/smooth-melons-study.md
new file mode 100644
index 000000000..356142f7e
--- /dev/null
+++ b/.changeset/smooth-melons-study.md
@@ -0,0 +1,5 @@
+---
+"@apollo/composition": patch
+---
+
+Type merging now uses maps instead of sparsely-populated arrays for per-subgraph data.