Skip to content

Commit 25ec49a

Browse files
committed
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`.
1 parent e2002dc commit 25ec49a

File tree

4 files changed

+133
-59
lines changed

4 files changed

+133
-59
lines changed

src/execution/execute.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
import { typeFromAST } from '../utilities/typeFromAST';
6262
import { getOperationRootType } from '../utilities/getOperationRootType';
6363

64+
import type { VariableValues } from './values';
6465
import {
6566
getVariableValues,
6667
getArgumentValues,
@@ -99,7 +100,7 @@ export interface ExecutionContext {
99100
rootValue: unknown;
100101
contextValue: unknown;
101102
operation: OperationDefinitionNode;
102-
variableValues: { [variable: string]: unknown };
103+
variableValues: VariableValues;
103104
fieldResolver: GraphQLFieldResolver<any, any>;
104105
typeResolver: GraphQLTypeResolver<any, any>;
105106
errors: Array<GraphQLError>;
@@ -302,15 +303,15 @@ export function buildExecutionContext(
302303
// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
303304
const variableDefinitions = operation.variableDefinitions ?? [];
304305

305-
const coercedVariableValues = getVariableValues(
306+
const variableValuesOrErrors = getVariableValues(
306307
schema,
307308
variableDefinitions,
308309
rawVariableValues ?? {},
309310
{ maxErrors: 50 },
310311
);
311312

312-
if (coercedVariableValues.errors) {
313-
return coercedVariableValues.errors;
313+
if (variableValuesOrErrors.errors) {
314+
return variableValuesOrErrors.errors;
314315
}
315316

316317
return {
@@ -319,7 +320,7 @@ export function buildExecutionContext(
319320
rootValue,
320321
contextValue,
321322
operation,
322-
variableValues: coercedVariableValues.coerced,
323+
variableValues: variableValuesOrErrors.variableValues,
323324
fieldResolver: fieldResolver ?? defaultFieldResolver,
324325
typeResolver: typeResolver ?? defaultTypeResolver,
325326
errors: [],
@@ -682,7 +683,7 @@ export function buildResolveInfo(
682683
fragments: exeContext.fragments,
683684
rootValue: exeContext.rootValue,
684685
operation: exeContext.operation,
685-
variableValues: exeContext.variableValues,
686+
variableValues: exeContext.variableValues.coerced,
686687
};
687688
}
688689

src/execution/values.ts

+38-22
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { ObjMap } from '../jsutils/ObjMap';
1+
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';
22
import type { Maybe } from '../jsutils/Maybe';
33
import { keyMap } from '../jsutils/keyMap';
44
import { inspect } from '../jsutils/inspect';
@@ -15,7 +15,7 @@ import { Kind } from '../language/kinds';
1515
import { print } from '../language/printer';
1616

1717
import type { GraphQLSchema } from '../type/schema';
18-
import type { GraphQLField } from '../type/definition';
18+
import type { GraphQLInputType, GraphQLField } from '../type/definition';
1919
import type { GraphQLDirective } from '../type/directives';
2020
import { isInputType, isNonNullType } from '../type/definition';
2121

@@ -26,9 +26,20 @@ import {
2626
coerceDefaultValue,
2727
} from '../utilities/coerceInputValue';
2828

29-
type CoercedVariableValues =
30-
| { errors: ReadonlyArray<GraphQLError>; coerced?: never }
31-
| { coerced: { [variable: string]: unknown }; errors?: never };
29+
export interface VariableValues {
30+
readonly sources: ReadOnlyObjMap<VariableValueSource>;
31+
readonly coerced: ReadOnlyObjMap<unknown>;
32+
}
33+
34+
interface VariableValueSource {
35+
readonly variable: VariableDefinitionNode;
36+
readonly type: GraphQLInputType;
37+
readonly value: unknown;
38+
}
39+
40+
type VariableValuesOrErrors =
41+
| { variableValues: VariableValues; errors?: never }
42+
| { errors: ReadonlyArray<GraphQLError>; variableValues?: never };
3243

3344
/**
3445
* Prepares an object map of variableValues of the correct type based on the
@@ -46,11 +57,11 @@ export function getVariableValues(
4657
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
4758
inputs: { readonly [variable: string]: unknown },
4859
options?: { maxErrors?: number },
49-
): CoercedVariableValues {
50-
const errors = [];
60+
): VariableValuesOrErrors {
61+
const errors: Array<GraphQLError> = [];
5162
const maxErrors = options?.maxErrors;
5263
try {
53-
const coerced = coerceVariableValues(
64+
const variableValues = coerceVariableValues(
5465
schema,
5566
varDefNodes,
5667
inputs,
@@ -65,7 +76,7 @@ export function getVariableValues(
6576
);
6677

6778
if (errors.length === 0) {
68-
return { coerced };
79+
return { variableValues };
6980
}
7081
} catch (error) {
7182
errors.push(error);
@@ -79,8 +90,9 @@ function coerceVariableValues(
7990
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
8091
inputs: { readonly [variable: string]: unknown },
8192
onError: (error: GraphQLError) => void,
82-
): { [variable: string]: unknown } {
83-
const coercedValues: { [variable: string]: unknown } = {};
93+
): VariableValues {
94+
const sources: ObjMap<VariableValueSource> = Object.create(null);
95+
const coerced: ObjMap<unknown> = Object.create(null);
8496
for (const varDefNode of varDefNodes) {
8597
const varName = varDefNode.variable.name.value;
8698
const varType = typeFromAST(schema, varDefNode.type);
@@ -98,11 +110,14 @@ function coerceVariableValues(
98110
}
99111

100112
if (!hasOwnProperty(inputs, varName)) {
101-
if (varDefNode.defaultValue) {
102-
coercedValues[varName] = coerceInputLiteral(
103-
varDefNode.defaultValue,
104-
varType,
105-
);
113+
const defaultValue = varDefNode.defaultValue;
114+
if (defaultValue) {
115+
sources[varName] = {
116+
variable: varDefNode,
117+
type: varType,
118+
value: undefined,
119+
};
120+
coerced[varName] = coerceInputLiteral(defaultValue, varType);
106121
} else if (isNonNullType(varType)) {
107122
const varTypeStr = inspect(varType);
108123
onError(
@@ -127,7 +142,8 @@ function coerceVariableValues(
127142
continue;
128143
}
129144

130-
coercedValues[varName] = coerceInputValue(
145+
sources[varName] = { variable: varDefNode, type: varType, value };
146+
coerced[varName] = coerceInputValue(
131147
value,
132148
varType,
133149
(path, invalidValue, error) => {
@@ -150,7 +166,7 @@ function coerceVariableValues(
150166
);
151167
}
152168

153-
return coercedValues;
169+
return { sources, coerced };
154170
}
155171

156172
/**
@@ -166,7 +182,7 @@ function coerceVariableValues(
166182
export function getArgumentValues(
167183
def: GraphQLField<unknown, unknown> | GraphQLDirective,
168184
node: FieldNode | DirectiveNode,
169-
variableValues?: Maybe<ObjMap<unknown>>,
185+
variableValues?: Maybe<VariableValues>,
170186
): { [argument: string]: unknown } {
171187
const coercedValues: { [argument: string]: unknown } = {};
172188

@@ -202,7 +218,7 @@ export function getArgumentValues(
202218
const variableName = valueNode.name.value;
203219
if (
204220
variableValues == null ||
205-
!hasOwnProperty(variableValues, variableName)
221+
variableValues.coerced[variableName] === undefined
206222
) {
207223
if (argDef.defaultValue) {
208224
coercedValues[name] = coerceDefaultValue(
@@ -218,7 +234,7 @@ export function getArgumentValues(
218234
}
219235
continue;
220236
}
221-
isNull = variableValues[variableName] == null;
237+
isNull = variableValues.coerced[variableName] == null;
222238
}
223239

224240
if (isNull && isNonNullType(argType)) {
@@ -258,7 +274,7 @@ export function getArgumentValues(
258274
export function getDirectiveValues(
259275
directiveDef: GraphQLDirective,
260276
node: { readonly directives?: ReadonlyArray<DirectiveNode> },
261-
variableValues?: Maybe<ObjMap<unknown>>,
277+
variableValues?: Maybe<VariableValues>,
262278
): undefined | { [argument: string]: unknown } {
263279
// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
264280
const directiveNode = node.directives?.find(

src/utilities/__tests__/coerceInputValue-test.ts

+68-16
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

4-
import type { ObjMap } from '../../jsutils/ObjMap';
4+
import type { ReadOnlyObjMap } from '../../jsutils/ObjMap';
55
import { invariant } from '../../jsutils/invariant';
66
import { identityFunc } from '../../jsutils/identityFunc';
77

88
import { print } from '../../language/printer';
9-
import { parseValue } from '../../language/parser';
9+
import { parseValue, Parser } from '../../language/parser';
1010

1111
import type { GraphQLInputType } from '../../type/definition';
1212
import {
@@ -23,6 +23,10 @@ import {
2323
GraphQLScalarType,
2424
GraphQLInputObjectType,
2525
} from '../../type/definition';
26+
import { GraphQLSchema } from '../../type/schema';
27+
28+
import type { VariableValues } from '../../execution/values';
29+
import { getVariableValues } from '../../execution/values';
2630

2731
import {
2832
coerceInputValue,
@@ -450,20 +454,29 @@ describe('coerceInputLiteral', () => {
450454
valueText: string,
451455
type: GraphQLInputType,
452456
expected: unknown,
453-
variables?: ObjMap<unknown>,
457+
variableValues?: VariableValues,
454458
) {
455459
const ast = parseValue(valueText);
456-
const value = coerceInputLiteral(ast, type, variables);
460+
const value = coerceInputLiteral(ast, type, variableValues);
457461
expect(value).to.deep.equal(expected);
458462
}
459463

460464
function testWithVariables(
461-
variables: ObjMap<unknown>,
465+
variableDefs: string,
466+
inputs: ReadOnlyObjMap<unknown>,
462467
valueText: string,
463468
type: GraphQLInputType,
464469
expected: unknown,
465470
) {
466-
test(valueText, type, expected, variables);
471+
const parser = new Parser(variableDefs);
472+
parser.expectToken('<SOF>');
473+
const variableValuesOrErrors = getVariableValues(
474+
new GraphQLSchema({}),
475+
parser.parseVariableDefinitions(),
476+
inputs,
477+
);
478+
invariant(variableValuesOrErrors.variableValues);
479+
test(valueText, type, expected, variableValuesOrErrors.variableValues);
467480
}
468481

469482
it('converts according to input coercion rules', () => {
@@ -662,19 +675,55 @@ describe('coerceInputLiteral', () => {
662675

663676
it('accepts variable values assuming already coerced', () => {
664677
test('$var', GraphQLBoolean, undefined);
665-
testWithVariables({ var: true }, '$var', GraphQLBoolean, true);
666-
testWithVariables({ var: null }, '$var', GraphQLBoolean, null);
667-
testWithVariables({ var: null }, '$var', nonNullBool, undefined);
678+
testWithVariables(
679+
'($var: Boolean)',
680+
{ var: true },
681+
'$var',
682+
GraphQLBoolean,
683+
true,
684+
);
685+
testWithVariables(
686+
'($var: Boolean)',
687+
{ var: null },
688+
'$var',
689+
GraphQLBoolean,
690+
null,
691+
);
692+
testWithVariables(
693+
'($var: Boolean)',
694+
{ var: null },
695+
'$var',
696+
nonNullBool,
697+
undefined,
698+
);
668699
});
669700

670701
it('asserts variables are provided as items in lists', () => {
671702
test('[ $foo ]', listOfBool, [null]);
672703
test('[ $foo ]', listOfNonNullBool, undefined);
673-
testWithVariables({ foo: true }, '[ $foo ]', listOfNonNullBool, [true]);
704+
testWithVariables(
705+
'($foo: Boolean)',
706+
{ foo: true },
707+
'[ $foo ]',
708+
listOfNonNullBool,
709+
[true],
710+
);
674711
// Note: variables are expected to have already been coerced, so we
675712
// do not expect the singleton wrapping behavior for variables.
676-
testWithVariables({ foo: true }, '$foo', listOfNonNullBool, true);
677-
testWithVariables({ foo: [true] }, '$foo', listOfNonNullBool, [true]);
713+
testWithVariables(
714+
'($foo: Boolean)',
715+
{ foo: true },
716+
'$foo',
717+
listOfNonNullBool,
718+
true,
719+
);
720+
testWithVariables(
721+
'($foo: [Boolean])',
722+
{ foo: [true] },
723+
'$foo',
724+
listOfNonNullBool,
725+
[true],
726+
);
678727
});
679728

680729
it('omits input object fields for unprovided variables', () => {
@@ -683,10 +732,13 @@ describe('coerceInputLiteral', () => {
683732
requiredBool: true,
684733
});
685734
test('{ requiredBool: $foo }', testInputObj, undefined);
686-
testWithVariables({ foo: true }, '{ requiredBool: $foo }', testInputObj, {
687-
int: 42,
688-
requiredBool: true,
689-
});
735+
testWithVariables(
736+
'($foo: Boolean)',
737+
{ foo: true },
738+
'{ requiredBool: $foo }',
739+
testInputObj,
740+
{ int: 42, requiredBool: true },
741+
);
690742
});
691743
});
692744

0 commit comments

Comments
 (0)