diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 5d7f25e722..a0a6884748 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -61,6 +61,7 @@ import { import { typeFromAST } from '../utilities/typeFromAST'; import { getOperationRootType } from '../utilities/getOperationRootType'; +import type { VariableValues } from './values'; import { getVariableValues, getArgumentValues, @@ -99,7 +100,7 @@ export interface ExecutionContext { rootValue: unknown; contextValue: unknown; operation: OperationDefinitionNode; - variableValues: { [variable: string]: unknown }; + variableValues: VariableValues; fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; errors: Array; @@ -302,15 +303,15 @@ export function buildExecutionContext( // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203') 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 { @@ -319,7 +320,7 @@ export function buildExecutionContext( rootValue, contextValue, operation, - variableValues: coercedVariableValues.coerced, + variableValues: variableValuesOrErrors.variableValues, fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, errors: [], @@ -682,7 +683,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 index 7ce5bca3b3..b71d2d624d 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -1,4 +1,4 @@ -import type { ObjMap } from '../jsutils/ObjMap'; +import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap'; import type { Maybe } from '../jsutils/Maybe'; import { keyMap } from '../jsutils/keyMap'; import { inspect } from '../jsutils/inspect'; @@ -15,7 +15,7 @@ import { Kind } from '../language/kinds'; import { print } from '../language/printer'; import type { GraphQLSchema } from '../type/schema'; -import type { GraphQLField } from '../type/definition'; +import type { GraphQLInputType, GraphQLField } from '../type/definition'; import type { GraphQLDirective } from '../type/directives'; import { isInputType, isNonNullType } from '../type/definition'; @@ -26,9 +26,20 @@ import { coerceDefaultValue, } from '../utilities/coerceInputValue'; -type CoercedVariableValues = - | { errors: ReadonlyArray; coerced?: never } - | { coerced: { [variable: string]: unknown }; errors?: never }; +export interface VariableValues { + readonly sources: ReadOnlyObjMap; + readonly coerced: ReadOnlyObjMap; +} + +interface VariableValueSource { + readonly variable: VariableDefinitionNode; + readonly type: GraphQLInputType; + readonly value: unknown; +} + +type VariableValuesOrErrors = + | { variableValues: VariableValues; errors?: never } + | { errors: ReadonlyArray; variableValues?: never }; /** * Prepares an object map of variableValues of the correct type based on the @@ -46,11 +57,11 @@ export function getVariableValues( varDefNodes: ReadonlyArray, inputs: { readonly [variable: string]: unknown }, options?: { maxErrors?: number }, -): CoercedVariableValues { - const errors = []; +): VariableValuesOrErrors { + const errors: Array = []; const maxErrors = options?.maxErrors; try { - const coerced = coerceVariableValues( + const variableValues = coerceVariableValues( schema, varDefNodes, inputs, @@ -65,7 +76,7 @@ export function getVariableValues( ); if (errors.length === 0) { - return { coerced }; + return { variableValues }; } } catch (error) { errors.push(error); @@ -79,8 +90,9 @@ function coerceVariableValues( varDefNodes: ReadonlyArray, inputs: { readonly [variable: string]: unknown }, onError: (error: GraphQLError) => void, -): { [variable: string]: unknown } { - const coercedValues: { [variable: string]: unknown } = {}; +): VariableValues { + const sources: ObjMap = Object.create(null); + const coerced: ObjMap = Object.create(null); for (const varDefNode of varDefNodes) { const varName = varDefNode.variable.name.value; const varType = typeFromAST(schema, varDefNode.type); @@ -98,11 +110,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( @@ -125,7 +140,8 @@ function coerceVariableValues( continue; } - coercedValues[varName] = coerceInputValue( + sources[varName] = { variable: varDefNode, type: varType, value }; + coerced[varName] = coerceInputValue( value, varType, (path, invalidValue, error) => { @@ -148,7 +164,7 @@ function coerceVariableValues( ); } - return coercedValues; + return { sources, coerced }; } /** @@ -164,7 +180,7 @@ function coerceVariableValues( export function getArgumentValues( def: GraphQLField | GraphQLDirective, node: FieldNode | DirectiveNode, - variableValues?: Maybe>, + variableValues?: Maybe, ): { [argument: string]: unknown } { const coercedValues: { [argument: string]: unknown } = {}; @@ -199,7 +215,7 @@ export function getArgumentValues( const variableName = valueNode.name.value; if ( variableValues == null || - !hasOwnProperty(variableValues, variableName) + variableValues.coerced[variableName] === undefined ) { if (argDef.defaultValue) { coercedValues[name] = coerceDefaultValue( @@ -215,7 +231,7 @@ export function getArgumentValues( } continue; } - isNull = variableValues[variableName] == null; + isNull = variableValues.coerced[variableName] == null; } if (isNull && isNonNullType(argType)) { @@ -256,7 +272,7 @@ export function getArgumentValues( export function getDirectiveValues( directiveDef: GraphQLDirective, node: { readonly directives?: ReadonlyArray }, - variableValues?: Maybe>, + variableValues?: Maybe, ): undefined | { [argument: string]: unknown } { // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203') const directiveNode = node.directives?.find( diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 145c231972..cb3eab6847 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -1,12 +1,12 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; -import type { ObjMap } from '../../jsutils/ObjMap'; +import type { ReadOnlyObjMap } from '../../jsutils/ObjMap'; import { invariant } from '../../jsutils/invariant'; import { identityFunc } from '../../jsutils/identityFunc'; import { print } from '../../language/printer'; -import { parseValue } from '../../language/parser'; +import { parseValue, Parser } from '../../language/parser'; import type { GraphQLInputType } from '../../type/definition'; import { @@ -23,6 +23,10 @@ import { GraphQLScalarType, GraphQLInputObjectType, } from '../../type/definition'; +import { GraphQLSchema } from '../../type/schema'; + +import type { VariableValues } from '../../execution/values'; +import { getVariableValues } from '../../execution/values'; import { coerceInputValue, @@ -450,20 +454,29 @@ describe('coerceInputLiteral', () => { valueText: string, type: GraphQLInputType, expected: unknown, - variables?: ObjMap, + 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, + variableDefs: string, + inputs: ReadOnlyObjMap, valueText: string, type: GraphQLInputType, expected: unknown, ) { - test(valueText, type, expected, variables); + const parser = new Parser(variableDefs); + parser.expectToken(''); + const variableValuesOrErrors = getVariableValues( + new GraphQLSchema({}), + parser.parseVariableDefinitions(), + inputs, + ); + invariant(variableValuesOrErrors.variableValues); + test(valueText, type, expected, variableValuesOrErrors.variableValues); } it('converts according to input coercion rules', () => { @@ -662,19 +675,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', () => { @@ -683,10 +732,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 }, + ); }); }); diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 9c25caf62f..77aafbd900 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -1,5 +1,4 @@ import type { Maybe } from '../jsutils/Maybe'; -import type { ObjMap } from '../jsutils/ObjMap'; import type { Path } from '../jsutils/Path'; import { hasOwnProperty } from '../jsutils/hasOwnProperty'; import { inspect } from '../jsutils/inspect'; @@ -30,6 +29,8 @@ import { import type { ValueNode } from '../language/ast'; import { Kind } from '../language/kinds'; +import type { VariableValues } from '../execution/values'; + type OnErrorCB = ( path: ReadonlyArray, invalidValue: unknown, @@ -211,26 +212,26 @@ function coerceInputValueImpl( export function coerceInputLiteral( valueNode: ValueNode, type: GraphQLInputType, - variables?: Maybe>, + variableValues?: Maybe, ): unknown { if (valueNode.kind === Kind.VARIABLE) { - if (!variables || isMissingVariable(valueNode, variables)) { + if (!variableValues || isMissingVariable(valueNode, variableValues)) { return; // Invalid: intentionally return no value. } - const variableValue = variables[valueNode.name.value]; - if (variableValue === null && isNonNullType(type)) { + const coercedVariableValue = variableValues.coerced[valueNode.name.value]; + if (coercedVariableValue === null && isNonNullType(type)) { return; // Invalid: intentionally return no value. } // Note: This does no further checking that this variable is correct. // This assumes validated has checked this variable is of the correct type. - return variableValue; + return coercedVariableValue; } if (isNonNullType(type)) { if (valueNode.kind === Kind.NULL) { return; // Invalid: intentionally return no value. } - return coerceInputLiteral(valueNode, type.ofType, variables); + return coerceInputLiteral(valueNode, type.ofType, variableValues); } if (valueNode.kind === Kind.NULL) { @@ -240,7 +241,11 @@ export function coerceInputLiteral( if (isListType(type)) { if (valueNode.kind !== Kind.LIST) { // Lists accept a non-list value as a list of one. - const itemValue = coerceInputLiteral(valueNode, type.ofType, variables); + const itemValue = coerceInputLiteral( + valueNode, + type.ofType, + variableValues, + ); if (itemValue === undefined) { return; // Invalid: intentionally return no value. } @@ -248,10 +253,10 @@ export function coerceInputLiteral( } const coercedValue: Array = []; for (const itemNode of valueNode.values) { - let itemValue = coerceInputLiteral(itemNode, type.ofType, variables); + let itemValue = coerceInputLiteral(itemNode, type.ofType, variableValues); if (itemValue === undefined) { if ( - isMissingVariable(itemNode, variables) && + isMissingVariable(itemNode, variableValues) && !isNonNullType(type.ofType) ) { // A missing variable within a list is coerced to null. @@ -281,7 +286,7 @@ export function coerceInputLiteral( const fieldNodes = keyMap(valueNode.fields, (field) => field.name.value); for (const field of Object.values(fieldDefs)) { const fieldNode = fieldNodes[field.name]; - if (!fieldNode || isMissingVariable(fieldNode.value, variables)) { + if (!fieldNode || isMissingVariable(fieldNode.value, variableValues)) { if (isRequiredInputField(field)) { return; // Invalid: intentionally return no value. } @@ -295,7 +300,7 @@ export function coerceInputLiteral( const fieldValue = coerceInputLiteral( fieldNode.value, field.type, - variables, + variableValues, ); if (fieldValue === undefined) { return; // Invalid: intentionally return no value. @@ -309,7 +314,7 @@ export function coerceInputLiteral( const leafType = assertLeafType(type); try { - return leafType.parseLiteral(valueNode, variables); + return leafType.parseLiteral(valueNode, variableValues?.coerced); } catch (_error) { // Invalid: ignore error and intentionally return no value. } @@ -319,11 +324,11 @@ export function coerceInputLiteral( // in the set of variables. function isMissingVariable( valueNode: ValueNode, - variables: Maybe>, + variables: Maybe, ): boolean { return ( valueNode.kind === Kind.VARIABLE && - (variables == null || variables[valueNode.name.value] === undefined) + (variables == null || variables.coerced[valueNode.name.value] === undefined) ); }