separateOperations: distinguish query and fragment names

Fixes #2853

src/utilities/__tests__/separateOperations-test.js

@@ -158,4 +158,73 @@ describe('separateOperations', () => {
       `,
     });
   });
+
+  it('distinguish query and fragment names', () => {
+    const ast = parse(`
+      {
+        ...NameClash
+      }
+
+      fragment NameClash on T {
+        oneField
+      }
+
+      query NameClash {
+        ...ShouldBeSkippedInFirstQuery
+      }
+
+      fragment ShouldBeSkippedInFirstQuery on T {
+        twoField
+      }
+    `);
+
+    const separatedASTs = mapValue(separateOperations(ast), print);
+    expect(separatedASTs).to.deep.equal({
+      '': dedent`
+        {
+          ...NameClash
+        }
+
+        fragment NameClash on T {
+          oneField
+        }
+      `,
+      NameClash: dedent`
+        query NameClash {
+          ...ShouldBeSkippedInFirstQuery
+        }
+
+        fragment ShouldBeSkippedInFirstQuery on T {
+          twoField
+        }
+      `,
+    });
+  });
+
+  it('handles unknown fragments', () => {
+    const ast = parse(`
+      {
+        ...Unknown
+        ...Known
+      }
+
+      fragment Known on T {
+        someField
+      }
+    `);
+
+    const separatedASTs = mapValue(separateOperations(ast), print);
+    expect(separatedASTs).to.deep.equal({
+      '': dedent`
+        {
+          ...Unknown
+          ...Known
+        }
+
+        fragment Known on T {
+          someField
+        }
+      `,
+    });
+  });
 });

src/utilities/separateOperations.js

@@ -1,6 +1,10 @@
 import type { ObjMap } from '../jsutils/ObjMap';
 
-import type { DocumentNode, OperationDefinitionNode } from '../language/ast';
+import type {
+  DocumentNode,
+  OperationDefinitionNode,
+  SelectionSetNode,
+} from '../language/ast';
 import { Kind } from '../language/kinds';
 import { visit } from '../language/visitor';
 
@@ -13,36 +17,35 @@ import { visit } from '../language/visitor';
 export function separateOperations(
   documentAST: DocumentNode,
 ): ObjMap<DocumentNode> {
-  const operations = [];
+  const operations: Array<OperationDefinitionNode> = [];
   const depGraph: DepGraph = Object.create(null);
-  let fromName;
 
   // Populate metadata and build a dependency graph.
-  visit(documentAST, {
-    OperationDefinition(node) {
-      fromName = opName(node);
-      operations.push(node);
-    },
-    FragmentDefinition(node) {
-      fromName = node.name.value;
-    },
-    FragmentSpread(node) {
-      const toName = node.name.value;
-      let dependents = depGraph[fromName];
-      if (dependents === undefined) {
-        dependents = depGraph[fromName] = Object.create(null);
-      }
-      dependents[toName] = true;
-    },
-  });
+  for (const definitionNode of documentAST.definitions) {
+    switch (definitionNode.kind) {
+      case Kind.OPERATION_DEFINITION:
+        operations.push(definitionNode);
+        break;
+      case Kind.FRAGMENT_DEFINITION:
+        depGraph[definitionNode.name.value] = collectDependencies(
+          definitionNode.selectionSet,
+        );
+        break;
+    }
+  }
 
   // For each operation, produce a new synthesized AST which includes only what
   // is necessary for completing that operation.
   const separatedDocumentASTs = Object.create(null);
   for (const operation of operations) {
-    const operationName = opName(operation);
-    const dependencies = Object.create(null);
-    collectTransitiveDependencies(dependencies, depGraph, operationName);
+    const dependencies = new Set();
+
+    for (const fragmentName of collectDependencies(operation.selectionSet)) {
+      collectTransitiveDependencies(dependencies, depGraph, fragmentName);
+    }
+
+    // Provides the empty string for anonymous operations.
+    const operationName = operation.name ? operation.name.value : '';
 
     // The list of definition nodes to be included for this operation, sorted
     // to retain the same order as the original document.
@@ -52,35 +55,42 @@ export function separateOperations(
         (node) =>
           node === operation ||
           (node.kind === Kind.FRAGMENT_DEFINITION &&
-            dependencies[node.name.value]),
+            dependencies.has(node.name.value)),
       ),
     };
   }
 
   return separatedDocumentASTs;
 }
 
-type DepGraph = ObjMap<ObjMap<boolean>>;
-
-// Provides the empty string for anonymous operations.
-function opName(operation: OperationDefinitionNode): string {
-  return operation.name ? operation.name.value : '';
-}
+type DepGraph = ObjMap<Array<string>>;
 
 // From a dependency graph, collects a list of transitive dependencies by
 // recursing through a dependency graph.
 function collectTransitiveDependencies(
-  collected: ObjMap<boolean>,
+  collected: Set<string>,
   depGraph: DepGraph,
   fromName: string,
 ): void {
-  const immediateDeps = depGraph[fromName];
-  if (immediateDeps) {
-    for (const toName of Object.keys(immediateDeps)) {
-      if (!collected[toName]) {
-        collected[toName] = true;
+  if (!collected.has(fromName)) {
+    collected.add(fromName);
+
+    const immediateDeps = depGraph[fromName];
+    if (immediateDeps !== undefined) {
+      for (const toName of immediateDeps) {
         collectTransitiveDependencies(collected, depGraph, toName);
       }
     }
   }
 }
+
+function collectDependencies(selectionSet: SelectionSetNode): Array<string> {
+  const dependencies = [];
+
+  visit(selectionSet, {
+    FragmentSpread(node) {
+      dependencies.push(node.name.value);
+    },
+  });
+  return dependencies;
+}