polish(fragmentArgs): simplify fragment args execution

benchmark/fragment-arguments-benchmark.js

@@ -0,0 +1,48 @@
+import { execute } from 'graphql/execution/execute.js';
+import { parse } from 'graphql/language/parser.js';
+import { buildSchema } from 'graphql/utilities/buildASTSchema.js';
+
+const schema = buildSchema('type Query { field(echo: String!): [String] }');
+
+const integers = Array.from({ length: 100 }, (_, i) => i + 1);
+
+const document = parse(
+  `
+    query WithManyFragmentArguments(${integers
+      .map((i) => `$echo${i}: String`)
+      .join(', ')}) {
+      ${integers.map((i) => `echo${i}: field(echo: $echo${i})`).join('\n')}
+      ${integers.map((i) => `...EchoFragment${i}(echo: $echo${i})`).join('\n')}
+    }
+
+    ${integers
+      .map(
+        (i) =>
+          `fragment EchoFragment${i}($echo: String) on Query { echoFragment${i}: field(echo: $echo) }`,
+      )
+      .join('\n')}
+  `,
+  { experimentalFragmentArguments: true },
+);
+
+const variableValues = Object.create(null);
+for (const i of integers) {
+  variableValues[`echo${i}`] = `Echo ${i}`;
+}
+
+function field(_, args) {
+  return args.echo;
+}
+
+export const benchmark = {
+  name: 'Execute Operation with Fragment Arguments',
+  count: 10,
+  async measure() {
+    await execute({
+      schema,
+      document,
+      rootValue: { field },
+      variableValues,
+    });
+  },
+};

src/execution/collectFields.ts

@@ -32,15 +32,10 @@ export interface DeferUsage {
   parentDeferUsage: DeferUsage | undefined;
 }
 
-export interface FragmentVariables {
-  signatures: ObjMap<GraphQLVariableSignature>;
-  values: ObjMap<unknown>;
-}
-
 export interface FieldDetails {
   node: FieldNode;
   deferUsage?: DeferUsage | undefined;
-  fragmentVariables?: FragmentVariables | undefined;
+  scopedVariableValues?: { [variable: string]: unknown } | undefined;
 }
 
 export type FieldGroup = ReadonlyArray<FieldDetails>;
@@ -136,14 +131,14 @@ export function collectSubfields(
   for (const fieldDetail of fieldGroup) {
     const selectionSet = fieldDetail.node.selectionSet;
     if (selectionSet) {
-      const { deferUsage, fragmentVariables } = fieldDetail;
+      const { deferUsage, scopedVariableValues } = fieldDetail;
       collectFieldsImpl(
         context,
         selectionSet,
         subGroupedFieldSet,
         newDeferUsages,
         deferUsage,
-        fragmentVariables,
+        scopedVariableValues,
       );
     }
   }
@@ -161,33 +156,35 @@ function collectFieldsImpl(
   groupedFieldSet: AccumulatorMap<string, FieldDetails>,
   newDeferUsages: Array<DeferUsage>,
   deferUsage?: DeferUsage,
-  fragmentVariables?: FragmentVariables,
+  scopedVariableValues?: { [variable: string]: unknown },
 ): void {
   const {
     schema,
     fragments,
-    variableValues,
+    variableValues: operationVariableValues,
     runtimeType,
     operation,
     visitedFragmentNames,
   } = context;
 
+  const variableValues = scopedVariableValues ?? operationVariableValues;
+
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
       case Kind.FIELD: {
-        if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
+        if (!shouldIncludeNode(variableValues, selection)) {
           continue;
         }
         groupedFieldSet.add(getFieldEntryKey(selection), {
           node: selection,
           deferUsage,
-          fragmentVariables,
+          scopedVariableValues,
         });
         break;
       }
       case Kind.INLINE_FRAGMENT: {
         if (
-          !shouldIncludeNode(selection, variableValues, fragmentVariables) ||
+          !shouldIncludeNode(variableValues, selection) ||
           !doesFragmentConditionMatch(schema, selection, runtimeType)
         ) {
           continue;
@@ -196,7 +193,6 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
-          fragmentVariables,
           selection,
           deferUsage,
         );
@@ -208,7 +204,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
-            fragmentVariables,
+            scopedVariableValues,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
@@ -218,7 +214,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
-            fragmentVariables,
+            scopedVariableValues,
           );
         }
 
@@ -230,15 +226,14 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
-          fragmentVariables,
           selection,
           deferUsage,
         );
 
         if (
           !newDeferUsage &&
           (visitedFragmentNames.has(fragName) ||
-            !shouldIncludeNode(selection, variableValues, fragmentVariables))
+            !shouldIncludeNode(variableValues, selection))
         ) {
           continue;
         }
@@ -251,19 +246,12 @@ function collectFieldsImpl(
           continue;
         }
 
-        const fragmentVariableSignatures = fragment.variableSignatures;
-        let newFragmentVariables: FragmentVariables | undefined;
-        if (fragmentVariableSignatures) {
-          newFragmentVariables = {
-            signatures: fragmentVariableSignatures,
-            values: experimentalGetArgumentValues(
-              selection,
-              Object.values(fragmentVariableSignatures),
-              variableValues,
-              fragmentVariables,
-            ),
-          };
-        }
+        const newScopedVariableValues = getScopedVariableValues(
+          fragment.variableSignatures,
+          selection,
+          variableValues,
+          operationVariableValues,
+        );
 
         if (!newDeferUsage) {
           visitedFragmentNames.add(fragName);
@@ -273,7 +261,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
-            newFragmentVariables,
+            newScopedVariableValues,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
@@ -283,7 +271,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
-            newFragmentVariables,
+            newScopedVariableValues,
           );
         }
         break;
@@ -300,16 +288,10 @@ function collectFieldsImpl(
 function getDeferUsage(
   operation: OperationDefinitionNode,
   variableValues: { [variable: string]: unknown },
-  fragmentVariables: FragmentVariables | undefined,
   node: FragmentSpreadNode | InlineFragmentNode,
   parentDeferUsage: DeferUsage | undefined,
 ): DeferUsage | undefined {
-  const defer = getDirectiveValues(
-    GraphQLDeferDirective,
-    node,
-    variableValues,
-    fragmentVariables,
-  );
+  const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues);
 
   if (!defer) {
     return;
@@ -335,16 +317,10 @@ function getDeferUsage(
  * directives, where `@skip` has higher precedence than `@include`.
  */
 function shouldIncludeNode(
-  node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
   variableValues: { [variable: string]: unknown },
-  fragmentVariables: FragmentVariables | undefined,
+  node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
 ): boolean {
-  const skip = getDirectiveValues(
-    GraphQLSkipDirective,
-    node,
-    variableValues,
-    fragmentVariables,
-  );
+  const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);
   if (skip?.if === true) {
     return false;
   }
@@ -353,7 +329,6 @@ function shouldIncludeNode(
     GraphQLIncludeDirective,
     node,
     variableValues,
-    fragmentVariables,
   );
   if (include?.if === false) {
     return false;
@@ -383,6 +358,31 @@ function doesFragmentConditionMatch(
   return false;
 }
 
+/**
+ * Constructs a variableValues map for the given fragment.
+ */
+function getScopedVariableValues(
+  fragmentVariableSignatures: ObjMap<GraphQLVariableSignature> | undefined,
+  fragmentSpreadNode: FragmentSpreadNode,
+  variableValues: { [variable: string]: unknown },
+  operationVariableValues: { [variable: string]: unknown },
+): { [variable: string]: unknown } | undefined {
+  if (!fragmentVariableSignatures) {
+    return;
+  }
+  const scopedVariableValues = experimentalGetArgumentValues(
+    fragmentSpreadNode,
+    Object.values(fragmentVariableSignatures),
+    variableValues,
+  );
+  for (const [variableName, value] of Object.entries(operationVariableValues)) {
+    if (fragmentVariableSignatures[variableName] === undefined) {
+      scopedVariableValues[variableName] = value;
+    }
+  }
+  return scopedVariableValues;
+}
+
 /**
  * Implements the logic to compute the key of a given field's entry
  */

src/execution/execute.ts

@@ -752,8 +752,7 @@ function executeField(
     const args = experimentalGetArgumentValues(
       fieldGroup[0].node,
       fieldDef.args,
-      exeContext.variableValues,
-      fieldGroup[0].fragmentVariables,
+      fieldGroup[0].scopedVariableValues ?? exeContext.variableValues,
     );
 
     // The resolve function's optional third argument is a context value that
@@ -1061,8 +1060,7 @@ function getStreamUsage(
   const stream = getDirectiveValues(
     GraphQLStreamDirective,
     fieldGroup[0].node,
-    exeContext.variableValues,
-    fieldGroup[0].fragmentVariables,
+    fieldGroup[0].scopedVariableValues ?? exeContext.variableValues,
   );
 
   if (!stream) {
@@ -1091,7 +1089,7 @@ function getStreamUsage(
   const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({
     node: fieldDetails.node,
     deferUsage: undefined,
-    fragmentVariables: fieldDetails.fragmentVariables,
+    scopedVariableValues: fieldDetails.scopedVariableValues,
   }));
 
   const streamUsage = {

src/execution/values.ts

@@ -22,7 +22,6 @@ import type { GraphQLSchema } from '../type/schema.js';
 import { coerceInputValue } from '../utilities/coerceInputValue.js';
 import { valueFromAST } from '../utilities/valueFromAST.js';
 
-import type { FragmentVariables } from './collectFields.js';
 import type { GraphQLVariableSignature } from './getVariableSignature.js';
 import { getVariableSignature } from './getVariableSignature.js';
 
@@ -156,7 +155,6 @@ export function experimentalGetArgumentValues(
   node: FieldNode | DirectiveNode | FragmentSpreadNode,
   argDefs: ReadonlyArray<GraphQLArgument | GraphQLVariableSignature>,
   variableValues: Maybe<ObjMap<unknown>>,
-  fragmentVariables?: Maybe<FragmentVariables>,
 ): { [argument: string]: unknown } {
   const coercedValues: { [argument: string]: unknown } = {};
 
@@ -188,12 +186,9 @@ export function experimentalGetArgumentValues(
 
     if (valueNode.kind === Kind.VARIABLE) {
       const variableName = valueNode.name.value;
-      const scopedVariableValues = fragmentVariables?.signatures[variableName]
-        ? fragmentVariables.values
-        : variableValues;
       if (
-        scopedVariableValues == null ||
-        !Object.hasOwn(scopedVariableValues, variableName)
+        variableValues == null ||
+        !Object.hasOwn(variableValues, variableName)
       ) {
         if (argDef.defaultValue !== undefined) {
           coercedValues[name] = argDef.defaultValue;
@@ -206,7 +201,7 @@ export function experimentalGetArgumentValues(
         }
         continue;
       }
-      isNull = scopedVariableValues[variableName] == null;
+      isNull = variableValues[variableName] == null;
     }
 
     if (isNull && isNonNullType(argType)) {
@@ -217,12 +212,7 @@ export function experimentalGetArgumentValues(
       );
     }
 
-    const coercedValue = valueFromAST(
-      valueNode,
-      argType,
-      variableValues,
-      fragmentVariables?.values,
-    );
+    const coercedValue = valueFromAST(valueNode, argType, variableValues);
     if (coercedValue === undefined) {
       // Note: ValuesOfCorrectTypeRule validation should catch this before
       // execution. This is a runtime check to ensure execution does not
@@ -254,7 +244,6 @@ export function getDirectiveValues(
   directiveDef: GraphQLDirective,
   node: { readonly directives?: ReadonlyArray<DirectiveNode> | undefined },
   variableValues?: Maybe<ObjMap<unknown>>,
-  fragmentVariables?: Maybe<FragmentVariables>,
 ): undefined | { [argument: string]: unknown } {
   const directiveNode = node.directives?.find(
     (directive) => directive.name.value === directiveDef.name,
@@ -265,7 +254,6 @@ export function getDirectiveValues(
       directiveNode,
       directiveDef.args,
       variableValues,
-      fragmentVariables,
     );
   }
 }