Skip to content

Commit 12f67c0

Browse files
leebyronyaacovCR
leebyron
authored and
yaacovCR
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 cafef7b commit 12f67c0

6 files changed

Lines changed: 145 additions & 69 deletions

File tree

‎src/execution/collectFields.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type { GraphQLSchema } from '../type/schema.js';
2424

2525
import { typeFromAST } from '../utilities/typeFromAST.js';
2626

27+
import type { VariableValues } from './values.js';
2728
import { getDirectiveValues } from './values.js';
2829

2930
export interface PatchFields {
@@ -48,7 +49,7 @@ export interface FieldsAndPatches {
4849
export function collectFields(
4950
schema: GraphQLSchema,
5051
fragments: ObjMap<FragmentDefinitionNode>,
51-
variableValues: { [variable: string]: unknown },
52+
variableValues: VariableValues,
5253
runtimeType: GraphQLObjectType,
5354
operation: OperationDefinitionNode,
5455
): FieldsAndPatches {
@@ -82,7 +83,7 @@ export function collectFields(
8283
export function collectSubfields(
8384
schema: GraphQLSchema,
8485
fragments: ObjMap<FragmentDefinitionNode>,
85-
variableValues: { [variable: string]: unknown },
86+
variableValues: VariableValues,
8687
operation: OperationDefinitionNode,
8788
returnType: GraphQLObjectType,
8889
fieldNodes: ReadonlyArray<FieldNode>,
@@ -118,7 +119,7 @@ export function collectSubfields(
118119
function collectFieldsImpl(
119120
schema: GraphQLSchema,
120121
fragments: ObjMap<FragmentDefinitionNode>,
121-
variableValues: { [variable: string]: unknown },
122+
variableValues: VariableValues,
122123
operation: OperationDefinitionNode,
123124
runtimeType: GraphQLObjectType,
124125
selectionSet: SelectionSetNode,
@@ -244,7 +245,7 @@ function collectFieldsImpl(
244245
*/
245246
function getDeferValues(
246247
operation: OperationDefinitionNode,
247-
variableValues: { [variable: string]: unknown },
248+
variableValues: VariableValues,
248249
node: FragmentSpreadNode | InlineFragmentNode,
249250
): undefined | { label: string | undefined } {
250251
const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues);
@@ -272,7 +273,7 @@ function getDeferValues(
272273
* directives, where `@skip` has higher precedence than `@include`.
273274
*/
274275
function shouldIncludeNode(
275-
variableValues: { [variable: string]: unknown },
276+
variableValues: VariableValues,
276277
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
277278
): boolean {
278279
const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);

‎src/execution/execute.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
collectSubfields as _collectSubfields,
5454
} from './collectFields.js';
5555
import { mapAsyncIterable } from './mapAsyncIterable.js';
56+
import type { VariableValues } from './values';
5657
import {
5758
getArgumentValues,
5859
getDirectiveValues,
@@ -116,7 +117,7 @@ export interface ExecutionContext {
116117
rootValue: unknown;
117118
contextValue: unknown;
118119
operation: OperationDefinitionNode;
119-
variableValues: { [variable: string]: unknown };
120+
variableValues: VariableValues;
120121
fieldResolver: GraphQLFieldResolver<any, any>;
121122
typeResolver: GraphQLTypeResolver<any, any>;
122123
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
@@ -482,15 +483,15 @@ export function buildExecutionContext(
482483
/* c8 ignore next */
483484
const variableDefinitions = operation.variableDefinitions ?? [];
484485

485-
const coercedVariableValues = getVariableValues(
486+
const variableValuesOrErrors = getVariableValues(
486487
schema,
487488
variableDefinitions,
488489
rawVariableValues ?? {},
489490
{ maxErrors: 50 },
490491
);
491492

492-
if (coercedVariableValues.errors) {
493-
return coercedVariableValues.errors;
493+
if (variableValuesOrErrors.errors) {
494+
return variableValuesOrErrors.errors;
494495
}
495496

496497
return {
@@ -499,7 +500,7 @@ export function buildExecutionContext(
499500
rootValue,
500501
contextValue,
501502
operation,
502-
variableValues: coercedVariableValues.coerced,
503+
variableValues: variableValuesOrErrors.variableValues,
503504
fieldResolver: fieldResolver ?? defaultFieldResolver,
504505
typeResolver: typeResolver ?? defaultTypeResolver,
505506
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -787,7 +788,7 @@ export function buildResolveInfo(
787788
fragments: exeContext.fragments,
788789
rootValue: exeContext.rootValue,
789790
operation: exeContext.operation,
790-
variableValues: exeContext.variableValues,
791+
variableValues: exeContext.variableValues.coerced,
791792
};
792793
}
793794

‎src/execution/values.ts‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { inspect } from '../jsutils/inspect.js';
22
import { keyMap } from '../jsutils/keyMap.js';
33
import type { Maybe } from '../jsutils/Maybe.js';
4-
import type { ObjMap } from '../jsutils/ObjMap.js';
4+
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';
55
import { printPathArray } from '../jsutils/printPathArray.js';
66

77
import { GraphQLError } from '../error/GraphQLError.js';
@@ -14,7 +14,7 @@ import type {
1414
import { Kind } from '../language/kinds.js';
1515
import { print } from '../language/printer.js';
1616

17-
import type { GraphQLField } from '../type/definition.js';
17+
import type { GraphQLField, GraphQLInputType } from '../type/definition.js';
1818
import { isInputType, isNonNullType } from '../type/definition.js';
1919
import type { GraphQLDirective } from '../type/directives.js';
2020
import type { GraphQLSchema } from '../type/schema.js';
@@ -26,9 +26,20 @@ import {
2626
} from '../utilities/coerceInputValue.js';
2727
import { typeFromAST } from '../utilities/typeFromAST.js';
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
@@ -44,11 +55,11 @@ export function getVariableValues(
4455
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
4556
inputs: { readonly [variable: string]: unknown },
4657
options?: { maxErrors?: number },
47-
): CoercedVariableValues {
48-
const errors = [];
58+
): VariableValuesOrErrors {
59+
const errors: Array<GraphQLError> = [];
4960
const maxErrors = options?.maxErrors;
5061
try {
51-
const coerced = coerceVariableValues(
62+
const variableValues = coerceVariableValues(
5263
schema,
5364
varDefNodes,
5465
inputs,
@@ -63,7 +74,7 @@ export function getVariableValues(
6374
);
6475

6576
if (errors.length === 0) {
66-
return { coerced };
77+
return { variableValues };
6778
}
6879
} catch (error) {
6980
errors.push(error);
@@ -77,8 +88,9 @@ function coerceVariableValues(
7788
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
7889
inputs: { readonly [variable: string]: unknown },
7990
onError: (error: GraphQLError) => void,
80-
): { [variable: string]: unknown } {
81-
const coercedValues: { [variable: string]: unknown } = {};
91+
): VariableValues {
92+
const sources: ObjMap<VariableValueSource> = Object.create(null);
93+
const coerced: ObjMap<unknown> = Object.create(null);
8294
for (const varDefNode of varDefNodes) {
8395
const varName = varDefNode.variable.name.value;
8496
const varType = typeFromAST(schema, varDefNode.type);
@@ -96,11 +108,14 @@ function coerceVariableValues(
96108
}
97109

98110
if (!hasOwnProperty(inputs, varName)) {
99-
if (varDefNode.defaultValue) {
100-
coercedValues[varName] = coerceInputLiteral(
101-
varDefNode.defaultValue,
102-
varType,
103-
);
111+
const defaultValue = varDefNode.defaultValue;
112+
if (defaultValue) {
113+
sources[varName] = {
114+
variable: varDefNode,
115+
type: varType,
116+
value: undefined,
117+
};
118+
coerced[varName] = coerceInputLiteral(defaultValue, varType);
104119
} else if (isNonNullType(varType)) {
105120
onError(
106121
new GraphQLError(
@@ -123,7 +138,8 @@ function coerceVariableValues(
123138
continue;
124139
}
125140

126-
coercedValues[varName] = coerceInputValue(
141+
sources[varName] = { variable: varDefNode, type: varType, value };
142+
coerced[varName] = coerceInputValue(
127143
value,
128144
varType,
129145
(path, invalidValue, error) => {
@@ -142,7 +158,7 @@ function coerceVariableValues(
142158
);
143159
}
144160

145-
return coercedValues;
161+
return { sources, coerced };
146162
}
147163

148164
/**
@@ -156,7 +172,7 @@ function coerceVariableValues(
156172
export function getArgumentValues(
157173
def: GraphQLField<unknown, unknown> | GraphQLDirective,
158174
node: FieldNode | DirectiveNode,
159-
variableValues?: Maybe<ObjMap<unknown>>,
175+
variableValues?: Maybe<VariableValues>,
160176
): { [argument: string]: unknown } {
161177
const coercedValues: { [argument: string]: unknown } = {};
162178

@@ -192,7 +208,7 @@ export function getArgumentValues(
192208
const variableName = valueNode.name.value;
193209
if (
194210
variableValues == null ||
195-
!hasOwnProperty(variableValues, variableName)
211+
!hasOwnProperty(variableValues.coerced, variableName)
196212
) {
197213
if (argDef.defaultValue) {
198214
coercedValues[name] = coerceDefaultValue(
@@ -208,7 +224,7 @@ export function getArgumentValues(
208224
}
209225
continue;
210226
}
211-
isNull = variableValues[variableName] == null;
227+
isNull = variableValues.coerced[variableName] == null;
212228
}
213229

214230
if (isNull && isNonNullType(argType)) {
@@ -249,7 +265,7 @@ export function getArgumentValues(
249265
export function getDirectiveValues(
250266
directiveDef: GraphQLDirective,
251267
node: { readonly directives?: ReadonlyArray<DirectiveNode> | undefined },
252-
variableValues?: Maybe<ObjMap<unknown>>,
268+
variableValues?: Maybe<VariableValues>,
253269
): undefined | { [argument: string]: unknown } {
254270
const directiveNode = node.directives?.find(
255271
(directive) => directive.name.value === directiveDef.name,

‎src/utilities/__tests__/coerceInputValue-test.ts‎

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ import { describe, it } from 'mocha';
33

44
import { identityFunc } from '../../jsutils/identityFunc.js';
55
import { invariant } from '../../jsutils/invariant.js';
6-
import type { ObjMap } from '../../jsutils/ObjMap.js';
6+
import type { ReadOnlyObjMap } from '../../jsutils/ObjMap.js';
77

88
import { Kind } from '../../language/kinds.js';
9-
import { parseValue } from '../../language/parser.js';
9+
import { Parser, parseValue } from '../../language/parser.js';
1010
import { print } from '../../language/printer.js';
11+
import { TokenKind } from '../../language/tokenKind.js';
1112

1213
import type { GraphQLInputType } from '../../type/definition.js';
1314
import {
@@ -24,6 +25,10 @@ import {
2425
GraphQLInt,
2526
GraphQLString,
2627
} from '../../type/scalars.js';
28+
import { GraphQLSchema } from '../../type/schema.js';
29+
30+
import type { VariableValues } from '../../execution/values.js';
31+
import { getVariableValues } from '../../execution/values.js';
2732

2833
import {
2934
coerceDefaultValue,
@@ -451,20 +456,29 @@ describe('coerceInputLiteral', () => {
451456
valueText: string,
452457
type: GraphQLInputType,
453458
expected: unknown,
454-
variables?: ObjMap<unknown>,
459+
variableValues?: VariableValues,
455460
) {
456461
const ast = parseValue(valueText);
457-
const value = coerceInputLiteral(ast, type, variables);
462+
const value = coerceInputLiteral(ast, type, variableValues);
458463
expect(value).to.deep.equal(expected);
459464
}
460465

461466
function testWithVariables(
462-
variables: ObjMap<unknown>,
467+
variableDefs: string,
468+
inputs: ReadOnlyObjMap<unknown>,
463469
valueText: string,
464470
type: GraphQLInputType,
465471
expected: unknown,
466472
) {
467-
test(valueText, type, expected, variables);
473+
const parser = new Parser(variableDefs);
474+
parser.expectToken(TokenKind.SOF);
475+
const variableValuesOrErrors = getVariableValues(
476+
new GraphQLSchema({}),
477+
parser.parseVariableDefinitions(),
478+
inputs,
479+
);
480+
invariant(variableValuesOrErrors.variableValues !== undefined);
481+
test(valueText, type, expected, variableValuesOrErrors.variableValues);
468482
}
469483

470484
it('converts according to input coercion rules', () => {
@@ -663,19 +677,55 @@ describe('coerceInputLiteral', () => {
663677

664678
it('accepts variable values assuming already coerced', () => {
665679
test('$var', GraphQLBoolean, undefined);
666-
testWithVariables({ var: true }, '$var', GraphQLBoolean, true);
667-
testWithVariables({ var: null }, '$var', GraphQLBoolean, null);
668-
testWithVariables({ var: null }, '$var', nonNullBool, undefined);
680+
testWithVariables(
681+
'($var: Boolean)',
682+
{ var: true },
683+
'$var',
684+
GraphQLBoolean,
685+
true,
686+
);
687+
testWithVariables(
688+
'($var: Boolean)',
689+
{ var: null },
690+
'$var',
691+
GraphQLBoolean,
692+
null,
693+
);
694+
testWithVariables(
695+
'($var: Boolean)',
696+
{ var: null },
697+
'$var',
698+
nonNullBool,
699+
undefined,
700+
);
669701
});
670702

671703
it('asserts variables are provided as items in lists', () => {
672704
test('[ $foo ]', listOfBool, [null]);
673705
test('[ $foo ]', listOfNonNullBool, undefined);
674-
testWithVariables({ foo: true }, '[ $foo ]', listOfNonNullBool, [true]);
706+
testWithVariables(
707+
'($foo: Boolean)',
708+
{ foo: true },
709+
'[ $foo ]',
710+
listOfNonNullBool,
711+
[true],
712+
);
675713
// Note: variables are expected to have already been coerced, so we
676714
// do not expect the singleton wrapping behavior for variables.
677-
testWithVariables({ foo: true }, '$foo', listOfNonNullBool, true);
678-
testWithVariables({ foo: [true] }, '$foo', listOfNonNullBool, [true]);
715+
testWithVariables(
716+
'($foo: Boolean)',
717+
{ foo: true },
718+
'$foo',
719+
listOfNonNullBool,
720+
true,
721+
);
722+
testWithVariables(
723+
'($foo: [Boolean])',
724+
{ foo: [true] },
725+
'$foo',
726+
listOfNonNullBool,
727+
[true],
728+
);
679729
});
680730

681731
it('omits input object fields for unprovided variables', () => {
@@ -684,10 +734,13 @@ describe('coerceInputLiteral', () => {
684734
requiredBool: true,
685735
});
686736
test('{ requiredBool: $foo }', testInputObj, undefined);
687-
testWithVariables({ foo: true }, '{ requiredBool: $foo }', testInputObj, {
688-
int: 42,
689-
requiredBool: true,
690-
});
737+
testWithVariables(
738+
'($foo: Boolean)',
739+
{ foo: true },
740+
'{ requiredBool: $foo }',
741+
testInputObj,
742+
{ int: 42, requiredBool: true },
743+
);
691744
});
692745
});
693746

0 commit comments

Comments
 (0)