feat: add experimental support for parsing fragment arguments (#4015)

This is a rebase of #3847

This implements execution of Fragment Arguments, and more specifically
visiting, parsing and printing of fragment-spreads with arguments and
fragment definitions with variables, as described by the spec changes in
graphql/graphql-spec#1081. There are a few
amendments in terms of execution and keying the fragment-spreads, these
are reflected in mjmahone/graphql-spec#3

The purpose is to be able to independently review all the moving parts,
the stacked PR's will contain mentions of open feedback that was present
at the time.

- [execution changes](JoviDeCroock#2)
- [TypeInfo & validation
changes](JoviDeCroock#4)
- [validation changes in
isolation](JoviDeCroock#5)


CC @mjmahone the original author

---------

Co-authored-by: mjmahone <[email protected]>
Co-authored-by: Yaacov Rydzinski <[email protected]>

Author: 3 people

src/execution/__tests__/variables-test.ts

@@ -24,26 +24,39 @@ import type { GraphQLSchema } from '../type/schema.js';
 
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
-import { getDirectiveValues } from './values.js';
+import type { GraphQLVariableSignature } from './getVariableSignature.js';
+import { experimentalGetArgumentValues, getDirectiveValues } from './values.js';
 
 export interface DeferUsage {
   label: string | undefined;
   parentDeferUsage: DeferUsage | undefined;
 }
 
+export interface FragmentVariables {
+  signatures: ObjMap<GraphQLVariableSignature>;
+  values: ObjMap<unknown>;
+}
+
 export interface FieldDetails {
   node: FieldNode;
-  deferUsage: DeferUsage | undefined;
+  deferUsage?: DeferUsage | undefined;
+  fragmentVariables?: FragmentVariables | undefined;
 }
 
 export type FieldGroup = ReadonlyArray<FieldDetails>;
 
 export type GroupedFieldSet = ReadonlyMap<string, FieldGroup>;
 
+export interface FragmentDetails {
+  definition: FragmentDefinitionNode;
+  variableSignatures?: ObjMap<GraphQLVariableSignature> | undefined;
+}
+
 interface CollectFieldsContext {
   schema: GraphQLSchema;
-  fragments: ObjMap<FragmentDefinitionNode>;
+  fragments: ObjMap<FragmentDetails>;
   variableValues: { [variable: string]: unknown };
+  fragmentVariableValues?: FragmentVariables;
   operation: OperationDefinitionNode;
   runtimeType: GraphQLObjectType;
   visitedFragmentNames: Set<string>;
@@ -60,7 +73,7 @@ interface CollectFieldsContext {
  */
 export function collectFields(
   schema: GraphQLSchema,
-  fragments: ObjMap<FragmentDefinitionNode>,
+  fragments: ObjMap<FragmentDetails>,
   variableValues: { [variable: string]: unknown },
   runtimeType: GraphQLObjectType,
   operation: OperationDefinitionNode,
@@ -101,7 +114,7 @@ export function collectFields(
 // eslint-disable-next-line max-params
 export function collectSubfields(
   schema: GraphQLSchema,
-  fragments: ObjMap<FragmentDefinitionNode>,
+  fragments: ObjMap<FragmentDetails>,
   variableValues: { [variable: string]: unknown },
   operation: OperationDefinitionNode,
   returnType: GraphQLObjectType,
@@ -140,12 +153,14 @@ export function collectSubfields(
   };
 }
 
+// eslint-disable-next-line max-params
 function collectFieldsImpl(
   context: CollectFieldsContext,
   selectionSet: SelectionSetNode,
   groupedFieldSet: AccumulatorMap<string, FieldDetails>,
   newDeferUsages: Array<DeferUsage>,
   deferUsage?: DeferUsage,
+  fragmentVariables?: FragmentVariables,
 ): void {
   const {
     schema,
@@ -159,18 +174,19 @@ function collectFieldsImpl(
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
       case Kind.FIELD: {
-        if (!shouldIncludeNode(variableValues, selection)) {
+        if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
           continue;
         }
         groupedFieldSet.add(getFieldEntryKey(selection), {
           node: selection,
           deferUsage,
+          fragmentVariables,
         });
         break;
       }
       case Kind.INLINE_FRAGMENT: {
         if (
-          !shouldIncludeNode(variableValues, selection) ||
+          !shouldIncludeNode(selection, variableValues, fragmentVariables) ||
           !doesFragmentConditionMatch(schema, selection, runtimeType)
         ) {
           continue;
@@ -179,6 +195,7 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
+          fragmentVariables,
           selection,
           deferUsage,
         );
@@ -190,6 +207,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
+            fragmentVariables,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
@@ -199,6 +217,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
+            fragmentVariables,
           );
         }
 
@@ -210,42 +229,60 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
+          fragmentVariables,
           selection,
           deferUsage,
         );
 
         if (
           !newDeferUsage &&
           (visitedFragmentNames.has(fragName) ||
-            !shouldIncludeNode(variableValues, selection))
+            !shouldIncludeNode(selection, variableValues, fragmentVariables))
         ) {
           continue;
         }
 
         const fragment = fragments[fragName];
         if (
           fragment == null ||
-          !doesFragmentConditionMatch(schema, fragment, runtimeType)
+          !doesFragmentConditionMatch(schema, fragment.definition, runtimeType)
         ) {
           continue;
         }
+
+        const fragmentVariableSignatures = fragment.variableSignatures;
+        let newFragmentVariables: FragmentVariables | undefined;
+        if (fragmentVariableSignatures) {
+          newFragmentVariables = {
+            signatures: fragmentVariableSignatures,
+            values: experimentalGetArgumentValues(
+              selection,
+              Object.values(fragmentVariableSignatures),
+              variableValues,
+              fragmentVariables,
+            ),
+          };
+        }
+
         if (!newDeferUsage) {
           visitedFragmentNames.add(fragName);
           collectFieldsImpl(
             context,
-            fragment.selectionSet,
+            fragment.definition.selectionSet,
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
+            newFragmentVariables,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
           collectFieldsImpl(
             context,
-            fragment.selectionSet,
+            fragment.definition.selectionSet,
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
+            newFragmentVariables,
           );
         }
         break;
@@ -262,10 +299,16 @@ 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);
+  const defer = getDirectiveValues(
+    GraphQLDeferDirective,
+    node,
+    variableValues,
+    fragmentVariables,
+  );
 
   if (!defer) {
     return;
@@ -291,10 +334,16 @@ function getDeferUsage(
  * directives, where `@skip` has higher precedence than `@include`.
  */
 function shouldIncludeNode(
-  variableValues: { [variable: string]: unknown },
   node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
+  variableValues: { [variable: string]: unknown },
+  fragmentVariables: FragmentVariables | undefined,
 ): boolean {
-  const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);
+  const skip = getDirectiveValues(
+    GraphQLSkipDirective,
+    node,
+    variableValues,
+    fragmentVariables,
+  );
   if (skip?.if === true) {
     return false;
   }
@@ -303,6 +352,7 @@ function shouldIncludeNode(
     GraphQLIncludeDirective,
     node,
     variableValues,
+    fragmentVariables,
   );
   if (include?.if === false) {
     return false;

src/execution/collectFields.ts

@@ -5,6 +5,7 @@ import { isAsyncIterable } from '../jsutils/isAsyncIterable.js';
 import { isIterableObject } from '../jsutils/isIterableObject.js';
 import { isObjectLike } from '../jsutils/isObjectLike.js';
 import { isPromise } from '../jsutils/isPromise.js';
+import { mapValue } from '../jsutils/mapValue.js';
 import type { Maybe } from '../jsutils/Maybe.js';
 import { memoize3 } from '../jsutils/memoize3.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
@@ -20,7 +21,6 @@ import { locatedError } from '../error/locatedError.js';
 import type {
   DocumentNode,
   FieldNode,
-  FragmentDefinitionNode,
   OperationDefinitionNode,
 } from '../language/ast.js';
 import { OperationTypeNode } from '../language/ast.js';
@@ -53,12 +53,14 @@ import { buildExecutionPlan } from './buildExecutionPlan.js';
 import type {
   DeferUsage,
   FieldGroup,
+  FragmentDetails,
   GroupedFieldSet,
 } from './collectFields.js';
 import {
   collectFields,
   collectSubfields as _collectSubfields,
 } from './collectFields.js';
+import { getVariableSignature } from './getVariableSignature.js';
 import { buildIncrementalResponse } from './IncrementalPublisher.js';
 import { mapAsyncIterable } from './mapAsyncIterable.js';
 import type {
@@ -74,6 +76,7 @@ import type {
 } from './types.js';
 import { DeferredFragmentRecord } from './types.js';
 import {
+  experimentalGetArgumentValues,
   getArgumentValues,
   getDirectiveValues,
   getVariableValues,
@@ -132,7 +135,7 @@ const collectSubfields = memoize3(
  */
 export interface ExecutionContext {
   schema: GraphQLSchema;
-  fragments: ObjMap<FragmentDefinitionNode>;
+  fragments: ObjMap<FragmentDetails>;
   rootValue: unknown;
   contextValue: unknown;
   operation: OperationDefinitionNode;
@@ -462,7 +465,7 @@ export function buildExecutionContext(
   assertValidSchema(schema);
 
   let operation: OperationDefinitionNode | undefined;
-  const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
+  const fragments: ObjMap<FragmentDetails> = Object.create(null);
   for (const definition of document.definitions) {
     switch (definition.kind) {
       case Kind.OPERATION_DEFINITION:
@@ -479,9 +482,18 @@ export function buildExecutionContext(
           operation = definition;
         }
         break;
-      case Kind.FRAGMENT_DEFINITION:
-        fragments[definition.name.value] = definition;
+      case Kind.FRAGMENT_DEFINITION: {
+        let variableSignatures;
+        if (definition.variableDefinitions) {
+          variableSignatures = Object.create(null);
+          for (const varDef of definition.variableDefinitions) {
+            const signature = getVariableSignature(schema, varDef);
+            variableSignatures[signature.name] = signature;
+          }
+        }
+        fragments[definition.name.value] = { definition, variableSignatures };
         break;
+      }
       default:
       // ignore non-executable definitions
     }
@@ -737,10 +749,11 @@ function executeField(
     // Build a JS object of arguments from the field.arguments AST, using the
     // 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,
+    const args = experimentalGetArgumentValues(
       fieldGroup[0].node,
+      fieldDef.args,
       exeContext.variableValues,
+      fieldGroup[0].fragmentVariables,
     );
 
     // The resolve function's optional third argument is a context value that
@@ -823,7 +836,10 @@ export function buildResolveInfo(
     parentType,
     path,
     schema: exeContext.schema,
-    fragments: exeContext.fragments,
+    fragments: mapValue(
+      exeContext.fragments,
+      (fragment) => fragment.definition,
+    ),
     rootValue: exeContext.rootValue,
     operation: exeContext.operation,
     variableValues: exeContext.variableValues,
@@ -1046,6 +1062,7 @@ function getStreamUsage(
     GraphQLStreamDirective,
     fieldGroup[0].node,
     exeContext.variableValues,
+    fieldGroup[0].fragmentVariables,
   );
 
   if (!stream) {
@@ -1074,6 +1091,7 @@ function getStreamUsage(
   const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({
     node: fieldDetails.node,
     deferUsage: undefined,
+    fragmentVariables: fieldDetails.fragmentVariables,
   }));
 
   const streamUsage = {

src/execution/execute.ts

@@ -0,0 +1,46 @@
+import { GraphQLError } from '../error/GraphQLError.js';
+
+import type { VariableDefinitionNode } from '../language/ast.js';
+import { print } from '../language/printer.js';
+
+import { isInputType } from '../type/definition.js';
+import type { GraphQLInputType, GraphQLSchema } from '../type/index.js';
+
+import { typeFromAST } from '../utilities/typeFromAST.js';
+import { valueFromAST } from '../utilities/valueFromAST.js';
+
+/**
+ * A GraphQLVariableSignature is required to coerce a variable value.
+ *
+ * Designed to have comparable interface to GraphQLArgument so that
+ * getArgumentValues() can be reused for fragment arguments.
+ * */
+export interface GraphQLVariableSignature {
+  name: string;
+  type: GraphQLInputType;
+  defaultValue: unknown;
+}
+
+export function getVariableSignature(
+  schema: GraphQLSchema,
+  varDefNode: VariableDefinitionNode,
+): GraphQLVariableSignature | GraphQLError {
+  const varName = varDefNode.variable.name.value;
+  const varType = typeFromAST(schema, varDefNode.type);
+
+  if (!isInputType(varType)) {
+    // Must use input types for variables. This should be caught during
+    // validation, however is checked again here for safety.
+    const varTypeStr = print(varDefNode.type);
+    return new GraphQLError(
+      `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`,
+      { nodes: varDefNode.type },
+    );
+  }
+
+  return {
+    name: varName,
+    type: varType,
+    defaultValue: valueFromAST(varDefNode.defaultValue, varType),
+  };
+}