add fix for new tests

if we allow nesting of defers at the same level then we have to handle the case where a defer is completely empty, not just empty because its fields are also contained within a parent.

this means that collectFIelds must directly return the new defer usages rather than just set the stage for later iteration of the new fields

Author: yaacovCR

src/execution/buildFieldPlan.ts

@@ -8,7 +8,6 @@ export type DeferUsageSet = ReadonlySet<DeferUsage>;
 export interface FieldGroup {
   fields: ReadonlyArray<FieldDetails>;
   deferUsages?: DeferUsageSet | undefined;
-  knownDeferUsages?: DeferUsageSet | undefined;
 }
 
 export type GroupedFieldSet = Map<string, FieldGroup>;
@@ -21,21 +20,15 @@ export interface NewGroupedFieldSetDetails {
 export function buildFieldPlan(
   fields: Map<string, ReadonlyArray<FieldDetails>>,
   parentDeferUsages: DeferUsageSet = new Set<DeferUsage>(),
-  knownDeferUsages: DeferUsageSet = new Set<DeferUsage>(),
 ): {
   groupedFieldSet: GroupedFieldSet;
   newGroupedFieldSetDetailsMap: Map<DeferUsageSet, NewGroupedFieldSetDetails>;
-  newDeferUsages: ReadonlyArray<DeferUsage>;
 } {
-  const newDeferUsages: Set<DeferUsage> = new Set<DeferUsage>();
-  const newKnownDeferUsages = new Set<DeferUsage>(knownDeferUsages);
-
   const groupedFieldSet = new Map<
     string,
     {
       fields: Array<FieldDetails>;
       deferUsages: DeferUsageSet;
-      knownDeferUsages: DeferUsageSet;
     }
   >();
 
@@ -47,7 +40,6 @@ export function buildFieldPlan(
         {
           fields: Array<FieldDetails>;
           deferUsages: DeferUsageSet;
-          knownDeferUsages: DeferUsageSet;
         }
       >;
       shouldInitiateDefer: boolean;
@@ -72,10 +64,6 @@ export function buildFieldPlan(
         continue;
       }
       deferUsageSet.add(deferUsage);
-      if (!knownDeferUsages.has(deferUsage)) {
-        newDeferUsages.add(deferUsage);
-        newKnownDeferUsages.add(deferUsage);
-      }
     }
     if (inOriginalResult) {
       deferUsageSet.clear();
@@ -99,7 +87,6 @@ export function buildFieldPlan(
         fieldGroup = {
           fields: [],
           deferUsages: deferUsageSet,
-          knownDeferUsages: newKnownDeferUsages,
         };
         groupedFieldSet.set(responseKey, fieldGroup);
       }
@@ -140,7 +127,6 @@ export function buildFieldPlan(
       fieldGroup = {
         fields: [],
         deferUsages: deferUsageSet,
-        knownDeferUsages: newKnownDeferUsages,
       };
       newGroupedFieldSet.set(responseKey, fieldGroup);
     }
@@ -150,7 +136,6 @@ export function buildFieldPlan(
   return {
     groupedFieldSet,
     newGroupedFieldSetDetailsMap,
-    newDeferUsages: Array.from(newDeferUsages),
   };
 }
 

src/execution/collectFields.ts

@@ -60,8 +60,12 @@ export function collectFields(
   variableValues: { [variable: string]: unknown },
   runtimeType: GraphQLObjectType,
   operation: OperationDefinitionNode,
-): Map<string, ReadonlyArray<FieldDetails>> {
+): {
+  fields: Map<string, ReadonlyArray<FieldDetails>>;
+  newDeferUsages: ReadonlyArray<DeferUsage>;
+} {
   const groupedFieldSet = new AccumulatorMap<string, FieldDetails>();
+  const newDeferUsages: Array<DeferUsage> = [];
   const context: CollectFieldsContext = {
     schema,
     fragments,
@@ -71,8 +75,13 @@ export function collectFields(
     visitedFragmentNames: new Set(),
   };
 
-  collectFieldsImpl(context, operation.selectionSet, groupedFieldSet);
-  return groupedFieldSet;
+  collectFieldsImpl(
+    context,
+    operation.selectionSet,
+    groupedFieldSet,
+    newDeferUsages,
+  );
+  return { fields: groupedFieldSet, newDeferUsages };
 }
 
 /**
@@ -93,7 +102,10 @@ export function collectSubfields(
   operation: OperationDefinitionNode,
   returnType: GraphQLObjectType,
   fieldDetails: ReadonlyArray<FieldDetails>,
-): Map<string, ReadonlyArray<FieldDetails>> {
+): {
+  fields: Map<string, ReadonlyArray<FieldDetails>>;
+  newDeferUsages: ReadonlyArray<DeferUsage>;
+} {
   const context: CollectFieldsContext = {
     schema,
     fragments,
@@ -103,6 +115,7 @@ export function collectSubfields(
     visitedFragmentNames: new Set(),
   };
   const subGroupedFieldSet = new AccumulatorMap<string, FieldDetails>();
+  const newDeferUsages: Array<DeferUsage> = [];
 
   for (const fieldDetail of fieldDetails) {
     const node = fieldDetail.node;
@@ -111,18 +124,23 @@ export function collectSubfields(
         context,
         node.selectionSet,
         subGroupedFieldSet,
+        newDeferUsages,
         fieldDetail.deferUsage,
       );
     }
   }
 
-  return subGroupedFieldSet;
+  return {
+    fields: subGroupedFieldSet,
+    newDeferUsages,
+  };
 }
 
 function collectFieldsImpl(
   context: CollectFieldsContext,
   selectionSet: SelectionSetNode,
   groupedFieldSet: AccumulatorMap<string, FieldDetails>,
+  newDeferUsages: Array<DeferUsage>,
   deferUsage?: DeferUsage,
 ): void {
   const {
@@ -161,12 +179,24 @@ function collectFieldsImpl(
           deferUsage,
         );
 
-        collectFieldsImpl(
-          context,
-          selection.selectionSet,
-          groupedFieldSet,
-          newDeferUsage ?? deferUsage,
-        );
+        if (!newDeferUsage) {
+          collectFieldsImpl(
+            context,
+            selection.selectionSet,
+            groupedFieldSet,
+            newDeferUsages,
+            deferUsage,
+          );
+        } else {
+          newDeferUsages.push(newDeferUsage);
+          collectFieldsImpl(
+            context,
+            selection.selectionSet,
+            groupedFieldSet,
+            newDeferUsages,
+            newDeferUsage,
+          );
+        }
 
         break;
       }
@@ -197,14 +227,23 @@ function collectFieldsImpl(
         }
         if (!newDeferUsage) {
           visitedFragmentNames.add(fragName);
+          collectFieldsImpl(
+            context,
+            fragment.selectionSet,
+            groupedFieldSet,
+            newDeferUsages,
+            deferUsage,
+          );
+        } else {
+          newDeferUsages.push(newDeferUsage);
+          collectFieldsImpl(
+            context,
+            fragment.selectionSet,
+            groupedFieldSet,
+            newDeferUsages,
+            newDeferUsage,
+          );
         }
-
-        collectFieldsImpl(
-          context,
-          fragment.selectionSet,
-          groupedFieldSet,
-          newDeferUsage ?? deferUsage,
-        );
         break;
       }
     }

src/execution/execute.ts

@@ -91,19 +91,18 @@ const buildSubFieldPlan = memoize3(
     returnType: GraphQLObjectType,
     fieldGroup: FieldGroup,
   ) => {
-    const subFields = collectSubfields(
+    const { fields: subFields, newDeferUsages } = collectSubfields(
       exeContext.schema,
       exeContext.fragments,
       exeContext.variableValues,
       exeContext.operation,
       returnType,
       fieldGroup.fields,
     );
-    return buildFieldPlan(
-      subFields,
-      fieldGroup.deferUsages,
-      fieldGroup.knownDeferUsages,
-    );
+    return {
+      ...buildFieldPlan(subFields, fieldGroup.deferUsages),
+      newDeferUsages,
+    };
   },
 );
 
@@ -408,14 +407,14 @@ function executeOperation(
     );
   }
 
-  const fields = collectFields(
+  const { fields, newDeferUsages } = collectFields(
     schema,
     fragments,
     variableValues,
     rootType,
     operation,
   );
-  const { groupedFieldSet, newGroupedFieldSetDetailsMap, newDeferUsages } =
+  const { groupedFieldSet, newGroupedFieldSetDetailsMap } =
     buildFieldPlan(fields);
 
   const newDeferMap = addNewDeferredFragments(
@@ -1807,7 +1806,7 @@ function executeSubscription(
     );
   }
 
-  const fields = collectFields(
+  const { fields } = collectFields(
     schema,
     fragments,
     variableValues,

src/validation/rules/SingleFieldSubscriptionsRule.ts

@@ -49,7 +49,7 @@ export function SingleFieldSubscriptionsRule(
               fragments[definition.name.value] = definition;
             }
           }
-          const fields = collectFields(
+          const { fields } = collectFields(
             schema,
             fragments,
             variableValues,