hoist depth out of TaggedFieldNode into FieldGroup

Author: yaacovCR

src/execution/__tests__/executor-test.ts

@@ -241,7 +241,7 @@ describe('Execute: Handles basic execution tasks', () => {
 
     const fieldNode = operation.selectionSet.selections[0];
     expect(resolvedInfo).to.deep.include({
-      fieldGroup: [{ fieldNode, depth: 0, deferDepth: undefined }],
+      fieldGroup: { depth: 0, fields: [{ fieldNode, deferDepth: undefined }] },
       deferDepth: undefined,
       variableValues: { var: 'abc' },
     });

src/execution/collectFields.ts

@@ -1,4 +1,3 @@
-import { AccumulatorMap } from '../jsutils/AccumulatorMap.js';
 import { invariant } from '../jsutils/invariant.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
 
@@ -26,28 +25,38 @@ import { typeFromAST } from '../utilities/typeFromAST.js';
 
 import { getDirectiveValues } from './values.js';
 
-export type FieldGroup = ReadonlyArray<TaggedFieldNode>;
-
-export type GroupedFieldSet = Map<string, FieldGroup>;
-
 /**
- * A tagged field node includes metadata necessary to determine whether a field should
- * be executed.
+ * A field group is a list of fields with the same response key.
  *
- * A field's depth is equivalent to the number of fields between the given field and
+ * The group's depth is equivalent to the number of fields between the group and
  * the operation root. For example, root fields have a depth of 0, their sub-fields
- * have a depth of 1, and so on. Tagging fields with their depth is necessary only to
- * compute a field's "defer depth".
+ * have a depth of 1, and so on.
  *
- * A field's defer depth is the depth of the closest containing defer directive , or
- * undefined, if the field is not contained by a deferred fragment.
+ * The groups's depth is provided so that CollectField algorithm can compute the
+ * depth of an inline or named fragment with a defer directive.
+ */
+export interface FieldGroup {
+  depth: number;
+  fields: ReadonlyArray<TaggedFieldNode>;
+}
+
+export type GroupedFieldSet = Map<string, FieldGroup>;
+
+interface MutableFieldSet {
+  depth: number;
+  fields: Array<TaggedFieldNode>;
+}
+
+/**
+ * A tagged field node includes the depth of any enclosing defer directive, or
+ * undefined, if the field is not contained within a deferred fragment.
  *
- * Because deferred fragments at a given level are merged, the defer depth may be used
- * as a unique id to tag the fields for inclusion within a given deferred payload.
+ * Because deferred fragments at a given level are merged, the defer depth for a
+ * field node may be used as a unique id to tag the fields for inclusion within a
+ * given deferred payload.
  */
 export interface TaggedFieldNode {
   fieldNode: FieldNode;
-  depth: number;
   deferDepth: number | undefined;
 }
 
@@ -70,7 +79,7 @@ export function collectFields(
   groupedFieldSet: GroupedFieldSet;
   newDeferDepth: number | undefined;
 } {
-  const groupedFieldSet = new AccumulatorMap<string, TaggedFieldNode>();
+  const groupedFieldSet = new Map<string, MutableFieldSet>();
   const newDeferDepth = collectFieldsImpl(
     schema,
     fragments,
@@ -111,11 +120,11 @@ export function collectSubfields(
   groupedFieldSet: GroupedFieldSet;
   newDeferDepth: number | undefined;
 } {
-  const groupedFieldSet = new AccumulatorMap<string, TaggedFieldNode>();
+  const groupedFieldSet = new Map<string, MutableFieldSet>();
   let newDeferDepth: number | undefined;
   const visitedFragmentNames = new Set<string>();
 
-  for (const field of fieldGroup) {
+  for (const field of fieldGroup.fields) {
     if (field.fieldNode.selectionSet) {
       const nestedNewDeferDepth = collectFieldsImpl(
         schema,
@@ -126,7 +135,7 @@ export function collectSubfields(
         field.fieldNode.selectionSet,
         groupedFieldSet,
         visitedFragmentNames,
-        fieldGroup[0].depth + 1,
+        fieldGroup.depth + 1,
         field.deferDepth,
       );
       if (nestedNewDeferDepth !== undefined) {
@@ -149,7 +158,7 @@ function collectFieldsImpl(
   operation: OperationDefinitionNode,
   runtimeType: GraphQLObjectType,
   selectionSet: SelectionSetNode,
-  groupedFieldSet: AccumulatorMap<string, TaggedFieldNode>,
+  groupedFieldSet: Map<string, MutableFieldSet>,
   visitedFragmentNames: Set<string>,
   depth: number,
   deferDepth: number | undefined,
@@ -161,11 +170,19 @@ function collectFieldsImpl(
         if (!shouldIncludeNode(variableValues, selection)) {
           continue;
         }
-        groupedFieldSet.add(getFieldEntryKey(selection), {
-          fieldNode: selection,
-          depth,
-          deferDepth,
-        });
+        const key = getFieldEntryKey(selection);
+        const fieldSet = groupedFieldSet.get(key);
+        if (fieldSet) {
+          fieldSet.fields.push({
+            fieldNode: selection,
+            deferDepth,
+          });
+        } else {
+          groupedFieldSet.set(key, {
+            depth,
+            fields: [{ fieldNode: selection, deferDepth }],
+          });
+        }
         break;
       }
       case Kind.INLINE_FRAGMENT: {

src/execution/execute.ts

@@ -660,7 +660,7 @@ function shouldExecute(
   fieldGroup: FieldGroup,
   deferDepth?: number | undefined,
 ): boolean {
-  return fieldGroup.some(
+  return fieldGroup.fields.some(
     ({ deferDepth: fieldDeferDepth }) => fieldDeferDepth === deferDepth,
   );
 }
@@ -724,7 +724,7 @@ function executeFields(
 }
 
 function toNodes(fieldGroup: FieldGroup): ReadonlyArray<FieldNode> {
-  return fieldGroup.map(({ fieldNode }) => fieldNode);
+  return fieldGroup.fields.map(({ fieldNode }) => fieldNode);
 }
 /**
  * Implements the "Executing fields" section of the spec
@@ -741,7 +741,7 @@ function executeField(
   asyncPayloadRecord?: AsyncPayloadRecord,
 ): PromiseOrValue<unknown> {
   const errors = asyncPayloadRecord?.errors ?? exeContext.errors;
-  const fieldName = fieldGroup[0].fieldNode.name.value;
+  const fieldName = fieldGroup.fields[0].fieldNode.name.value;
   const fieldDef = exeContext.schema.getField(parentType, fieldName);
   if (!fieldDef) {
     return;
@@ -766,7 +766,7 @@ function executeField(
     // TODO: find a way to memoize, in case this field is within a List type.
     const args = getArgumentValues(
       fieldDef,
-      fieldGroup[0].fieldNode,
+      fieldGroup.fields[0].fieldNode,
       exeContext.variableValues,
     );
 
@@ -1045,7 +1045,7 @@ function getStreamValues(
   // safe to only check the first fieldNode for the stream directive
   const stream = getDirectiveValues(
     GraphQLStreamDirective,
-    fieldGroup[0].fieldNode,
+    fieldGroup.fields[0].fieldNode,
     exeContext.variableValues,
   );
 
@@ -1780,7 +1780,7 @@ function executeSubscription(
     FieldGroup,
   ];
   const [responseName, fieldGroup] = firstRootField;
-  const fieldName = fieldGroup[0].fieldNode.name.value;
+  const fieldName = fieldGroup.fields[0].fieldNode.name.value;
   const fieldDef = schema.getField(rootType, fieldName);
 
   if (!fieldDef) {
@@ -1807,7 +1807,7 @@ function executeSubscription(
     // variables scope to fulfill any variable references.
     const args = getArgumentValues(
       fieldDef,
-      fieldGroup[0].fieldNode,
+      fieldGroup.fields[0].fieldNode,
       variableValues,
     );
 

src/type/definition.ts

@@ -890,11 +890,13 @@ export type GraphQLFieldResolver<
 
 export interface GraphQLResolveInfo {
   readonly fieldName: string;
-  readonly fieldGroup: ReadonlyArray<{
-    fieldNode: FieldNode;
+  readonly fieldGroup: {
     depth: number;
-    deferDepth: number | undefined;
-  }>;
+    fields: ReadonlyArray<{
+      fieldNode: FieldNode;
+      deferDepth: number | undefined;
+    }>;
+  };
   readonly deferDepth: number | undefined;
   readonly returnType: GraphQLOutputType;
   readonly parentType: GraphQLObjectType;

src/validation/rules/SingleFieldSubscriptionsRule.ts

@@ -52,6 +52,7 @@ export function SingleFieldSubscriptionsRule(
             const fieldGroups = [...groupedFieldSet.values()];
             const extraFieldGroups = fieldGroups.slice(1);
             const extraFields = extraFieldGroups
+              .map(({ fields }) => fields)
               .flat()
               .map(({ fieldNode }) => fieldNode);
             context.reportError(
@@ -63,8 +64,10 @@ export function SingleFieldSubscriptionsRule(
               ),
             );
           }
-          for (const fieldSet of groupedFieldSet.values()) {
-            const fieldNodes = fieldSet.map(({ fieldNode }) => fieldNode);
+          for (const fieldGroup of groupedFieldSet.values()) {
+            const fieldNodes = fieldGroup.fields.map(
+              ({ fieldNode }) => fieldNode,
+            );
             const fieldName = fieldNodes[0].name.value;
             if (fieldName.startsWith('__')) {
               context.reportError(