another alternative

Author: JoviDeCroock

src/execution/__tests__/variables-test.ts

@@ -104,7 +104,6 @@ function fieldWithInputArg(
     type: GraphQLString,
     args: { input: inputArg },
     resolve(_, args) {
-      console.log(args);
       if ('input' in args) {
         return inspect(args.input);
       }
@@ -1199,11 +1198,9 @@ describe('Execute: Handles inputs', () => {
           fieldWithNullableStringInput(input: $value)
         }
       `);
-      expect(result).to.deep.equal({
-        data: {
-          fieldWithNullableStringInput: null,
-        },
-      });
+      
+      // TODO: Make more exact
+      expect(result).to.have.property('errors');
     });
 
     it('when the definition has a default and is provided', () => {
@@ -1238,6 +1235,25 @@ describe('Execute: Handles inputs', () => {
       });
     });
 
+    it('when a definition has a default, is not provided, and spreads another fragment', () => {
+      const result = executeQueryWithFragmentArguments(`
+        query {
+          ...a
+        }
+        fragment a($a: String! = "B") on TestType {
+          ...b(b: $a)
+        }
+        fragment b($b: String!) on TestType {
+          fieldWithNonNullableStringInput(input: $b)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"B"',
+        },
+      });
+    });
+
     it('when the definition has a non-nullable default and is provided null', () => {
       const result = executeQueryWithFragmentArguments(`
         query {
@@ -1248,14 +1264,8 @@ describe('Execute: Handles inputs', () => {
         }
       `);
 
-      // TODO: this seems wrong, our variable definition tells us that `value` is a required string
-      // however we pass in null and expect this to be stringified into the input argument of our field.
-      // in the spec this sais it should raise a field-error
-      expect(result).to.deep.equal({
-        data: {
-          fieldWithNullableStringInput: 'null',
-        },
-      });
+      // TODO: Make more exact
+      expect(result).to.have.property('errors');
     });
 
     it('when the definition has no default and is not provided', () => {

src/execution/collectFields.ts

@@ -1,6 +1,4 @@
-import { GraphQLError } from '../error/GraphQLError.js';
 import { AccumulatorMap } from '../jsutils/AccumulatorMap.js';
-import { inspect } from '../jsutils/inspect.js';
 import { invariant } from '../jsutils/invariant.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
 
@@ -11,13 +9,11 @@ import type {
   InlineFragmentNode,
   OperationDefinitionNode,
   SelectionSetNode,
-  ValueNode,
 } from '../language/ast.js';
 import { OperationTypeNode } from '../language/ast.js';
 import { Kind } from '../language/kinds.js';
 
 import type { GraphQLObjectType } from '../type/definition.js';
-import { isInputType } from '../type/definition.js';
 import { isAbstractType } from '../type/definition.js';
 import {
   GraphQLDeferDirective,
@@ -27,10 +23,8 @@ import {
 import type { GraphQLSchema } from '../type/schema.js';
 
 import { typeFromAST } from '../utilities/typeFromAST.js';
-import { valueFromAST } from '../utilities/valueFromAST.js';
-import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped.js';
 
-import { getDirectiveValues } from './values.js';
+import { getDirectiveValues, getArgumentValuesFromSpread } from './values.js';
 
 export interface DeferUsage {
   label: string | undefined;
@@ -224,57 +218,23 @@ function collectFieldsImpl(
         //   scope as that variable can still get used in spreads later on in the selectionSet.
         // - when a value is passed in through the fragment-spread we need to copy over the key-value
         //   into our variable-values.
-        if (fragment.variableDefinitions) {
-          const rawVariables: ObjMap<unknown> = {};
-
-          const argumentValueLookup = new Map<string, ValueNode>();
-          if (selection.arguments) {
-            for (const argument of selection.arguments) {
-              argumentValueLookup.set(argument.name.value, argument.value);
-            }
-          }
-
-          for (const variableDefinition of fragment.variableDefinitions) {
-            const variableName = variableDefinition.variable.name.value;
-            const value = argumentValueLookup.get(variableName);
-            if (value) {
-              const varType = typeFromAST(context.schema, variableDefinition.type);
-              if (varType && isInputType(varType)) {
-                const argumentValue = valueFromAST(value, varType, { ...variableValues, ...fragmentVariableValues });
-                if (argumentValue !== undefined) {
-                  rawVariables[variableName] = argumentValue
-                  continue;
-                } else {
-                  throw new GraphQLError(
-                    `Argument "${variableName}" of required type "${inspect(varType)}" ` +
-                      'was not provided.',
-                    { nodes: selection },
-                  );
-                }
-              }
-            } else if (variableDefinition.defaultValue) {
-              rawVariables[variableName] = valueFromASTUntyped(variableDefinition.defaultValue, { ...variableValues, ...fragmentVariableValues });
-            }
-          }
-
-          collectFieldsImpl(
-            context,
-            fragment.selectionSet,
-            groupedFieldSet,
-            rawVariables,
-            parentDeferUsage,
-            newDeferUsage ?? deferUsage,
-          );
-        } else {
-          collectFieldsImpl(
-            context,
-            fragment.selectionSet,
-            groupedFieldSet,
-            undefined,
-            parentDeferUsage,
-            newDeferUsage ?? deferUsage,
-          );
-        }
+        const fragmentArgValues = fragment.variableDefinitions ? getArgumentValuesFromSpread(
+          selection,
+          schema,
+          fragment.variableDefinitions,
+          variableValues,
+          fragmentVariableValues,
+        ) : undefined;
+
+        collectFieldsImpl(
+          context,
+          fragment.selectionSet,
+          groupedFieldSet,
+          fragmentArgValues,
+          parentDeferUsage,
+          newDeferUsage ?? deferUsage,
+        );
+
         break;
       }
     }

src/execution/execute.ts

@@ -624,9 +624,10 @@ function executeField(
     // variables scope to fulfill any variable references.
     // TODO: find a way to memoize, in case this field is within a List type.
     const args = getArgumentValues(
-      fieldDef,
       fieldGroup.fields[0].node,
-      fieldGroup.fields[0].fragmentVariableValues ?? exeContext.variableValues,
+      fieldDef.args,
+      exeContext.variableValues,
+      fieldGroup.fields[0].fragmentVariableValues
     );
 
     // The resolve function's optional third argument is a context value that
@@ -1846,7 +1847,7 @@ function executeSubscription(
 
     // Build a JS object of arguments from the field.arguments AST, using the
     // variables scope to fulfill any variable references.
-    const args = getArgumentValues(fieldDef, fieldNodes[0], variableValues);
+    const args = getArgumentValues(fieldNodes[0], fieldDef.args, variableValues, undefined);
 
     // The resolve function's optional third argument is a context value that
     // is provided to every resolve function within an execution. It is commonly