Preserve sources of variable values (#3811)

[#3077 rebased on main](#3077). Depends on #3810 @leebyron comments from original PR: > By way of introducing type `VariableValues`, allows `getVariableValues` to return both the coerced values as well as the original sources, which are then made available in `ExecutionContext`. > > While variable sources are not used directly here, they're used directly in #3065. This PR is pulled out as a pre-req to aid review --------- Co-authored-by: Lee Byron <lee.byron@robinhood.com>
graphql · Sep 29, 2024 · 7dd825e · 7dd825e
1 parent 992b768
commit 7dd825e
Show file tree

Hide file tree

Showing 8 changed files with 230 additions and 115 deletions.
diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts
@@ -240,7 +240,19 @@ describe('Execute: Handles basic execution tasks', () => {
     expect(resolvedInfo).to.deep.include({
       fieldNodes: [field],
       path: { prev: undefined, key: 'result', typename: 'Test' },
-      variableValues: { var: 'abc' },
+      variableValues: {
+        sources: {
+          var: {
+            signature: {
+              name: 'var',
+              type: GraphQLString,
+              defaultValue: undefined,
+            },
+            value: 'abc',
+          },
+        },
+        coerced: { var: 'abc' },
+      },
     });
   });
 

diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts
@@ -25,22 +25,18 @@ import type { GraphQLSchema } from '../type/schema.js';
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
 import type { GraphQLVariableSignature } from './getVariableSignature.js';
-import { experimentalGetArgumentValues, getDirectiveValues } from './values.js';
+import type { VariableValues } from './values.js';
+import { getDirectiveValues, getFragmentVariableValues } 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;
-  fragmentVariables?: FragmentVariables | undefined;
+  fragmentVariableValues?: VariableValues | undefined;
 }
 
 export type FieldGroup = ReadonlyArray<FieldDetails>;
@@ -55,7 +51,7 @@ export interface FragmentDetails {
 interface CollectFieldsContext {
   schema: GraphQLSchema;
   fragments: ObjMap<FragmentDetails>;
-  variableValues: { [variable: string]: unknown };
+  variableValues: VariableValues;
   operation: OperationDefinitionNode;
   runtimeType: GraphQLObjectType;
   visitedFragmentNames: Set<string>;
@@ -73,7 +69,7 @@ interface CollectFieldsContext {
 export function collectFields(
   schema: GraphQLSchema,
   fragments: ObjMap<FragmentDetails>,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   runtimeType: GraphQLObjectType,
   operation: OperationDefinitionNode,
 ): {
@@ -114,7 +110,7 @@ export function collectFields(
 export function collectSubfields(
   schema: GraphQLSchema,
   fragments: ObjMap<FragmentDetails>,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   operation: OperationDefinitionNode,
   returnType: GraphQLObjectType,
   fieldGroup: FieldGroup,
@@ -136,14 +132,14 @@ export function collectSubfields(
   for (const fieldDetail of fieldGroup) {
     const selectionSet = fieldDetail.node.selectionSet;
     if (selectionSet) {
-      const { deferUsage, fragmentVariables } = fieldDetail;
+      const { deferUsage, fragmentVariableValues } = fieldDetail;
       collectFieldsImpl(
         context,
         selectionSet,
         subGroupedFieldSet,
         newDeferUsages,
         deferUsage,
-        fragmentVariables,
+        fragmentVariableValues,
       );
     }
   }
@@ -161,7 +157,7 @@ function collectFieldsImpl(
   groupedFieldSet: AccumulatorMap<string, FieldDetails>,
   newDeferUsages: Array<DeferUsage>,
   deferUsage?: DeferUsage,
-  fragmentVariables?: FragmentVariables,
+  fragmentVariableValues?: VariableValues,
 ): void {
   const {
     schema,
@@ -175,19 +171,25 @@ function collectFieldsImpl(
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
       case Kind.FIELD: {
-        if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
+        if (
+          !shouldIncludeNode(selection, variableValues, fragmentVariableValues)
+        ) {
           continue;
         }
         groupedFieldSet.add(getFieldEntryKey(selection), {
           node: selection,
           deferUsage,
-          fragmentVariables,
+          fragmentVariableValues,
         });
         break;
       }
       case Kind.INLINE_FRAGMENT: {
         if (
-          !shouldIncludeNode(selection, variableValues, fragmentVariables) ||
+          !shouldIncludeNode(
+            selection,
+            variableValues,
+            fragmentVariableValues,
+          ) ||
           !doesFragmentConditionMatch(schema, selection, runtimeType)
         ) {
           continue;
@@ -196,7 +198,7 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
-          fragmentVariables,
+          fragmentVariableValues,
           selection,
           deferUsage,
         );
@@ -208,7 +210,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
-            fragmentVariables,
+            fragmentVariableValues,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
@@ -218,7 +220,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
-            fragmentVariables,
+            fragmentVariableValues,
           );
         }
 
@@ -230,15 +232,19 @@ function collectFieldsImpl(
         const newDeferUsage = getDeferUsage(
           operation,
           variableValues,
-          fragmentVariables,
+          fragmentVariableValues,
           selection,
           deferUsage,
         );
 
         if (
           !newDeferUsage &&
           (visitedFragmentNames.has(fragName) ||
-            !shouldIncludeNode(selection, variableValues, fragmentVariables))
+            !shouldIncludeNode(
+              selection,
+              variableValues,
+              fragmentVariableValues,
+            ))
         ) {
           continue;
         }
@@ -252,17 +258,14 @@ function collectFieldsImpl(
         }
 
         const fragmentVariableSignatures = fragment.variableSignatures;
-        let newFragmentVariables: FragmentVariables | undefined;
+        let newFragmentVariableValues: VariableValues | undefined;
         if (fragmentVariableSignatures) {
-          newFragmentVariables = {
-            signatures: fragmentVariableSignatures,
-            values: experimentalGetArgumentValues(
-              selection,
-              Object.values(fragmentVariableSignatures),
-              variableValues,
-              fragmentVariables,
-            ),
-          };
+          newFragmentVariableValues = getFragmentVariableValues(
+            selection,
+            fragmentVariableSignatures,
+            variableValues,
+            fragmentVariableValues,
+          );
         }
 
         if (!newDeferUsage) {
@@ -273,7 +276,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             deferUsage,
-            newFragmentVariables,
+            newFragmentVariableValues,
           );
         } else {
           newDeferUsages.push(newDeferUsage);
@@ -283,7 +286,7 @@ function collectFieldsImpl(
             groupedFieldSet,
             newDeferUsages,
             newDeferUsage,
-            newFragmentVariables,
+            newFragmentVariableValues,
           );
         }
         break;
@@ -299,16 +302,16 @@ function collectFieldsImpl(
  */
 function getDeferUsage(
   operation: OperationDefinitionNode,
-  variableValues: { [variable: string]: unknown },
-  fragmentVariables: FragmentVariables | undefined,
+  variableValues: VariableValues,
+  fragmentVariableValues: VariableValues | undefined,
   node: FragmentSpreadNode | InlineFragmentNode,
   parentDeferUsage: DeferUsage | undefined,
 ): DeferUsage | undefined {
   const defer = getDirectiveValues(
     GraphQLDeferDirective,
     node,
     variableValues,
-    fragmentVariables,
+    fragmentVariableValues,
   );
 
   if (!defer) {
@@ -336,14 +339,14 @@ function getDeferUsage(
  */
 function shouldIncludeNode(
   node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
-  variableValues: { [variable: string]: unknown },
-  fragmentVariables: FragmentVariables | undefined,
+  variableValues: VariableValues,
+  fragmentVariableValues: VariableValues | undefined,
 ): boolean {
   const skip = getDirectiveValues(
     GraphQLSkipDirective,
     node,
     variableValues,
-    fragmentVariables,
+    fragmentVariableValues,
   );
   if (skip?.if === true) {
     return false;
@@ -353,7 +356,7 @@ function shouldIncludeNode(
     GraphQLIncludeDirective,
     node,
     variableValues,
-    fragmentVariables,
+    fragmentVariableValues,
   );
   if (include?.if === false) {
     return false;

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -75,6 +75,7 @@ import type {
   StreamRecord,
 } from './types.js';
 import { DeferredFragmentRecord } from './types.js';
+import type { VariableValues } from './values.js';
 import {
   experimentalGetArgumentValues,
   getArgumentValues,
@@ -139,7 +140,7 @@ export interface ExecutionContext {
   rootValue: unknown;
   contextValue: unknown;
   operation: OperationDefinitionNode;
-  variableValues: { [variable: string]: unknown };
+  variableValues: VariableValues;
   fieldResolver: GraphQLFieldResolver<any, any>;
   typeResolver: GraphQLTypeResolver<any, any>;
   subscribeFieldResolver: GraphQLFieldResolver<any, any>;
@@ -510,15 +511,15 @@ export function buildExecutionContext(
   /* c8 ignore next */
   const variableDefinitions = operation.variableDefinitions ?? [];
 
-  const coercedVariableValues = getVariableValues(
+  const variableValuesOrErrors = getVariableValues(
     schema,
     variableDefinitions,
     rawVariableValues ?? {},
     { maxErrors: 50 },
   );
 
-  if (coercedVariableValues.errors) {
-    return coercedVariableValues.errors;
+  if (variableValuesOrErrors.errors) {
+    return variableValuesOrErrors.errors;
   }
 
   return {
@@ -527,7 +528,7 @@ export function buildExecutionContext(
     rootValue,
     contextValue,
     operation,
-    variableValues: coercedVariableValues.coerced,
+    variableValues: variableValuesOrErrors.variableValues,
     fieldResolver: fieldResolver ?? defaultFieldResolver,
     typeResolver: typeResolver ?? defaultTypeResolver,
     subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -753,7 +754,7 @@ function executeField(
       fieldGroup[0].node,
       fieldDef.args,
       exeContext.variableValues,
-      fieldGroup[0].fragmentVariables,
+      fieldGroup[0].fragmentVariableValues,
     );
 
     // The resolve function's optional third argument is a context value that
@@ -1062,7 +1063,7 @@ function getStreamUsage(
     GraphQLStreamDirective,
     fieldGroup[0].node,
     exeContext.variableValues,
-    fieldGroup[0].fragmentVariables,
+    fieldGroup[0].fragmentVariableValues,
   );
 
   if (!stream) {
@@ -1091,7 +1092,7 @@ function getStreamUsage(
   const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({
     node: fieldDetails.node,
     deferUsage: undefined,
-    fragmentVariables: fieldDetails.fragmentVariables,
+    fragmentVariablesValues: fieldDetails.fragmentVariableValues,
   }));
 
   const streamUsage = {