Preserve sources of variable values

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`.
graphql · Feb 6, 2023 · d8feffe · d8feffe
1 parent 7a927c5
commit d8feffe
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 69 deletions.
diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts
@@ -24,6 +24,7 @@ import type { GraphQLSchema } from '../type/schema.js';
 
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
+import type { VariableValues } from './values.js';
 import { getDirectiveValues } from './values.js';
 
 export interface PatchFields {
@@ -48,7 +49,7 @@ export interface FieldsAndPatches {
 export function collectFields(
   schema: GraphQLSchema,
   fragments: ObjMap<FragmentDefinitionNode>,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   runtimeType: GraphQLObjectType,
   operation: OperationDefinitionNode,
 ): FieldsAndPatches {
@@ -82,7 +83,7 @@ export function collectFields(
 export function collectSubfields(
   schema: GraphQLSchema,
   fragments: ObjMap<FragmentDefinitionNode>,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   operation: OperationDefinitionNode,
   returnType: GraphQLObjectType,
   fieldNodes: ReadonlyArray<FieldNode>,
@@ -118,7 +119,7 @@ export function collectSubfields(
 function collectFieldsImpl(
   schema: GraphQLSchema,
   fragments: ObjMap<FragmentDefinitionNode>,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   operation: OperationDefinitionNode,
   runtimeType: GraphQLObjectType,
   selectionSet: SelectionSetNode,
@@ -244,7 +245,7 @@ function collectFieldsImpl(
  */
 function getDeferValues(
   operation: OperationDefinitionNode,
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   node: FragmentSpreadNode | InlineFragmentNode,
 ): undefined | { label: string | undefined } {
   const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues);
@@ -272,7 +273,7 @@ function getDeferValues(
  * directives, where `@skip` has higher precedence than `@include`.
  */
 function shouldIncludeNode(
-  variableValues: { [variable: string]: unknown },
+  variableValues: VariableValues,
   node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
 ): boolean {
   const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -53,6 +53,7 @@ import {
   collectSubfields as _collectSubfields,
 } from './collectFields.js';
 import { mapAsyncIterable } from './mapAsyncIterable.js';
+import type { VariableValues } from './values';
 import {
   getArgumentValues,
   getDirectiveValues,
@@ -116,7 +117,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>;
@@ -482,15 +483,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 {
@@ -499,7 +500,7 @@ export function buildExecutionContext(
     rootValue,
     contextValue,
     operation,
-    variableValues: coercedVariableValues.coerced,
+    variableValues: variableValuesOrErrors.variableValues,
     fieldResolver: fieldResolver ?? defaultFieldResolver,
     typeResolver: typeResolver ?? defaultTypeResolver,
     subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -787,7 +788,7 @@ export function buildResolveInfo(
     fragments: exeContext.fragments,
     rootValue: exeContext.rootValue,
     operation: exeContext.operation,
-    variableValues: exeContext.variableValues,
+    variableValues: exeContext.variableValues.coerced,
   };
 }
 

diff --git a/src/execution/values.ts b/src/execution/values.ts
@@ -1,7 +1,7 @@
 import { inspect } from '../jsutils/inspect.js';
 import { keyMap } from '../jsutils/keyMap.js';
 import type { Maybe } from '../jsutils/Maybe.js';
-import type { ObjMap } from '../jsutils/ObjMap.js';
+import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';
 import { printPathArray } from '../jsutils/printPathArray.js';
 
 import { GraphQLError } from '../error/GraphQLError.js';
@@ -14,7 +14,7 @@ import type {
 import { Kind } from '../language/kinds.js';
 import { print } from '../language/printer.js';
 
-import type { GraphQLField } from '../type/definition.js';
+import type { GraphQLField, GraphQLInputType } from '../type/definition.js';
 import { isInputType, isNonNullType } from '../type/definition.js';
 import type { GraphQLDirective } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
@@ -26,9 +26,20 @@ import {
 } from '../utilities/coerceInputValue.js';
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
-type CoercedVariableValues =
-  | { errors: ReadonlyArray<GraphQLError>; coerced?: never }
-  | { coerced: { [variable: string]: unknown }; errors?: never };
+export interface VariableValues {
+  readonly sources: ReadOnlyObjMap<VariableValueSource>;
+  readonly coerced: ReadOnlyObjMap<unknown>;
+}
+
+interface VariableValueSource {
+  readonly variable: VariableDefinitionNode;
+  readonly type: GraphQLInputType;
+  readonly value: unknown;
+}
+
+type VariableValuesOrErrors =
+  | { variableValues: VariableValues; errors?: never }
+  | { errors: ReadonlyArray<GraphQLError>; variableValues?: never };
 
 /**
  * Prepares an object map of variableValues of the correct type based on the
@@ -44,11 +55,11 @@ export function getVariableValues(
   varDefNodes: ReadonlyArray<VariableDefinitionNode>,
   inputs: { readonly [variable: string]: unknown },
   options?: { maxErrors?: number },
-): CoercedVariableValues {
-  const errors = [];
+): VariableValuesOrErrors {
+  const errors: Array<GraphQLError> = [];
   const maxErrors = options?.maxErrors;
   try {
-    const coerced = coerceVariableValues(
+    const variableValues = coerceVariableValues(
       schema,
       varDefNodes,
       inputs,
@@ -63,7 +74,7 @@ export function getVariableValues(
     );
 
     if (errors.length === 0) {
-      return { coerced };
+      return { variableValues };
     }
   } catch (error) {
     errors.push(error);
@@ -77,8 +88,9 @@ function coerceVariableValues(
   varDefNodes: ReadonlyArray<VariableDefinitionNode>,
   inputs: { readonly [variable: string]: unknown },
   onError: (error: GraphQLError) => void,
-): { [variable: string]: unknown } {
-  const coercedValues: { [variable: string]: unknown } = {};
+): VariableValues {
+  const sources: ObjMap<VariableValueSource> = Object.create(null);
+  const coerced: ObjMap<unknown> = Object.create(null);
   for (const varDefNode of varDefNodes) {
     const varName = varDefNode.variable.name.value;
     const varType = typeFromAST(schema, varDefNode.type);
@@ -96,11 +108,14 @@ function coerceVariableValues(
     }
 
     if (!hasOwnProperty(inputs, varName)) {
-      if (varDefNode.defaultValue) {
-        coercedValues[varName] = coerceInputLiteral(
-          varDefNode.defaultValue,
-          varType,
-        );
+      const defaultValue = varDefNode.defaultValue;
+      if (defaultValue) {
+        sources[varName] = {
+          variable: varDefNode,
+          type: varType,
+          value: undefined,
+        };
+        coerced[varName] = coerceInputLiteral(defaultValue, varType);
       } else if (isNonNullType(varType)) {
         onError(
           new GraphQLError(
@@ -123,7 +138,8 @@ function coerceVariableValues(
       continue;
     }
 
-    coercedValues[varName] = coerceInputValue(
+    sources[varName] = { variable: varDefNode, type: varType, value };
+    coerced[varName] = coerceInputValue(
       value,
       varType,
       (path, invalidValue, error) => {
@@ -142,7 +158,7 @@ function coerceVariableValues(
     );
   }
 
-  return coercedValues;
+  return { sources, coerced };
 }
 
 /**
@@ -156,7 +172,7 @@ function coerceVariableValues(
 export function getArgumentValues(
   def: GraphQLField<unknown, unknown> | GraphQLDirective,
   node: FieldNode | DirectiveNode,
-  variableValues?: Maybe<ObjMap<unknown>>,
+  variableValues?: Maybe<VariableValues>,
 ): { [argument: string]: unknown } {
   const coercedValues: { [argument: string]: unknown } = {};
 
@@ -192,7 +208,7 @@ export function getArgumentValues(
       const variableName = valueNode.name.value;
       if (
         variableValues == null ||
-        !hasOwnProperty(variableValues, variableName)
+        !hasOwnProperty(variableValues.coerced, variableName)
       ) {
         if (argDef.defaultValue) {
           coercedValues[name] = coerceDefaultValue(
@@ -208,7 +224,7 @@ export function getArgumentValues(
         }
         continue;
       }
-      isNull = variableValues[variableName] == null;
+      isNull = variableValues.coerced[variableName] == null;
     }
 
     if (isNull && isNonNullType(argType)) {
@@ -249,7 +265,7 @@ export function getArgumentValues(
 export function getDirectiveValues(
   directiveDef: GraphQLDirective,
   node: { readonly directives?: ReadonlyArray<DirectiveNode> | undefined },
-  variableValues?: Maybe<ObjMap<unknown>>,
+  variableValues?: Maybe<VariableValues>,
 ): undefined | { [argument: string]: unknown } {
   const directiveNode = node.directives?.find(
     (directive) => directive.name.value === directiveDef.name,

diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts
@@ -3,11 +3,12 @@ import { describe, it } from 'mocha';
 
 import { identityFunc } from '../../jsutils/identityFunc.js';
 import { invariant } from '../../jsutils/invariant.js';
-import type { ObjMap } from '../../jsutils/ObjMap.js';
+import type { ReadOnlyObjMap } from '../../jsutils/ObjMap.js';
 
 import { Kind } from '../../language/kinds.js';
-import { parseValue } from '../../language/parser.js';
+import { Parser, parseValue } from '../../language/parser.js';
 import { print } from '../../language/printer.js';
+import { TokenKind } from '../../language/tokenKind.js';
 
 import type { GraphQLInputType } from '../../type/definition.js';
 import {
@@ -24,6 +25,10 @@ import {
   GraphQLInt,
   GraphQLString,
 } from '../../type/scalars.js';
+import { GraphQLSchema } from '../../type/schema.js';
+
+import type { VariableValues } from '../../execution/values.js';
+import { getVariableValues } from '../../execution/values.js';
 
 import {
   coerceDefaultValue,
@@ -451,20 +456,29 @@ describe('coerceInputLiteral', () => {
     valueText: string,
     type: GraphQLInputType,
     expected: unknown,
-    variables?: ObjMap<unknown>,
+    variableValues?: VariableValues,
   ) {
     const ast = parseValue(valueText);
-    const value = coerceInputLiteral(ast, type, variables);
+    const value = coerceInputLiteral(ast, type, variableValues);
     expect(value).to.deep.equal(expected);
   }
 
   function testWithVariables(
-    variables: ObjMap<unknown>,
+    variableDefs: string,
+    inputs: ReadOnlyObjMap<unknown>,
     valueText: string,
     type: GraphQLInputType,
     expected: unknown,
   ) {
-    test(valueText, type, expected, variables);
+    const parser = new Parser(variableDefs);
+    parser.expectToken(TokenKind.SOF);
+    const variableValuesOrErrors = getVariableValues(
+      new GraphQLSchema({}),
+      parser.parseVariableDefinitions(),
+      inputs,
+    );
+    invariant(variableValuesOrErrors.variableValues !== undefined);
+    test(valueText, type, expected, variableValuesOrErrors.variableValues);
   }
 
   it('converts according to input coercion rules', () => {
@@ -663,19 +677,55 @@ describe('coerceInputLiteral', () => {
 
   it('accepts variable values assuming already coerced', () => {
     test('$var', GraphQLBoolean, undefined);
-    testWithVariables({ var: true }, '$var', GraphQLBoolean, true);
-    testWithVariables({ var: null }, '$var', GraphQLBoolean, null);
-    testWithVariables({ var: null }, '$var', nonNullBool, undefined);
+    testWithVariables(
+      '($var: Boolean)',
+      { var: true },
+      '$var',
+      GraphQLBoolean,
+      true,
+    );
+    testWithVariables(
+      '($var: Boolean)',
+      { var: null },
+      '$var',
+      GraphQLBoolean,
+      null,
+    );
+    testWithVariables(
+      '($var: Boolean)',
+      { var: null },
+      '$var',
+      nonNullBool,
+      undefined,
+    );
   });
 
   it('asserts variables are provided as items in lists', () => {
     test('[ $foo ]', listOfBool, [null]);
     test('[ $foo ]', listOfNonNullBool, undefined);
-    testWithVariables({ foo: true }, '[ $foo ]', listOfNonNullBool, [true]);
+    testWithVariables(
+      '($foo: Boolean)',
+      { foo: true },
+      '[ $foo ]',
+      listOfNonNullBool,
+      [true],
+    );
     // Note: variables are expected to have already been coerced, so we
     // do not expect the singleton wrapping behavior for variables.
-    testWithVariables({ foo: true }, '$foo', listOfNonNullBool, true);
-    testWithVariables({ foo: [true] }, '$foo', listOfNonNullBool, [true]);
+    testWithVariables(
+      '($foo: Boolean)',
+      { foo: true },
+      '$foo',
+      listOfNonNullBool,
+      true,
+    );
+    testWithVariables(
+      '($foo: [Boolean])',
+      { foo: [true] },
+      '$foo',
+      listOfNonNullBool,
+      [true],
+    );
   });
 
   it('omits input object fields for unprovided variables', () => {
@@ -684,10 +734,13 @@ describe('coerceInputLiteral', () => {
       requiredBool: true,
     });
     test('{ requiredBool: $foo }', testInputObj, undefined);
-    testWithVariables({ foo: true }, '{ requiredBool: $foo }', testInputObj, {
-      int: 42,
-      requiredBool: true,
-    });
+    testWithVariables(
+      '($foo: Boolean)',
+      { foo: true },
+      '{ requiredBool: $foo }',
+      testInputObj,
+      { int: 42, requiredBool: true },
+    );
   });
 });