Skip to content

Commit d8486ae

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 c3fc46d commit d8486ae

6 files changed

Lines changed: 144 additions & 68 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 type FieldGroup = ReadonlyArray<FieldNode>;
@@ -52,7 +53,7 @@ export interface FieldsAndPatches {
5253
export function collectFields(
5354
schema: GraphQLSchema,
5455
fragments: ObjMap<FragmentDefinitionNode>,
55-
variableValues: { [variable: string]: unknown },
56+
variableValues: VariableValues,
5657
runtimeType: GraphQLObjectType,
5758
operation: OperationDefinitionNode,
5859
): FieldsAndPatches {
@@ -86,7 +87,7 @@ export function collectFields(
8687
export function collectSubfields(
8788
schema: GraphQLSchema,
8889
fragments: ObjMap<FragmentDefinitionNode>,
89-
variableValues: { [variable: string]: unknown },
90+
variableValues: VariableValues,
9091
operation: OperationDefinitionNode,
9192
returnType: GraphQLObjectType,
9293
fieldGroup: FieldGroup,
@@ -122,7 +123,7 @@ export function collectSubfields(
122123
function collectFieldsImpl(
123124
schema: GraphQLSchema,
124125
fragments: ObjMap<FragmentDefinitionNode>,
125-
variableValues: { [variable: string]: unknown },
126+
variableValues: VariableValues,
126127
operation: OperationDefinitionNode,
127128
runtimeType: GraphQLObjectType,
128129
selectionSet: SelectionSetNode,
@@ -248,7 +249,7 @@ function collectFieldsImpl(
248249
*/
249250
function getDeferValues(
250251
operation: OperationDefinitionNode,
251-
variableValues: { [variable: string]: unknown },
252+
variableValues: VariableValues,
252253
node: FragmentSpreadNode | InlineFragmentNode,
253254
): undefined | { label: string | undefined } {
254255
const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues);
@@ -276,7 +277,7 @@ function getDeferValues(
276277
* directives, where `@skip` has higher precedence than `@include`.
277278
*/
278279
function shouldIncludeNode(
279-
variableValues: { [variable: string]: unknown },
280+
variableValues: VariableValues,
280281
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
281282
): boolean {
282283
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,
@@ -810,7 +811,7 @@ export function buildResolveInfo(
810811
fragments: exeContext.fragments,
811812
rootValue: exeContext.rootValue,
812813
operation: exeContext.operation,
813-
variableValues: exeContext.variableValues,
814+
variableValues: exeContext.variableValues.coerced,
814815
};
815816
}
816817

‎src/execution/values.ts‎

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

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

16-
import type { GraphQLField } from '../type/definition.js';
16+
import type { GraphQLField, GraphQLInputType } from '../type/definition.js';
1717
import { isInputType, isNonNullType } from '../type/definition.js';
1818
import type { GraphQLDirective } from '../type/directives.js';
1919
import type { GraphQLSchema } from '../type/schema.js';
@@ -25,9 +25,20 @@ import {
2525
} from '../utilities/coerceInputValue.js';
2626
import { typeFromAST } from '../utilities/typeFromAST.js';
2727

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

3243
/**
3344
* Prepares an object map of variableValues of the correct type based on the
@@ -43,11 +54,11 @@ export function getVariableValues(
4354
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
4455
inputs: { readonly [variable: string]: unknown },
4556
options?: { maxErrors?: number },
46-
): CoercedVariableValues {
47-
const errors = [];
57+
): VariableValuesOrErrors {
58+
const errors: Array<GraphQLError> = [];
4859
const maxErrors = options?.maxErrors;
4960
try {
50-
const coerced = coerceVariableValues(
61+
const variableValues = coerceVariableValues(
5162
schema,
5263
varDefNodes,
5364
inputs,
@@ -62,7 +73,7 @@ export function getVariableValues(
6273
);
6374

6475
if (errors.length === 0) {
65-
return { coerced };
76+
return { variableValues };
6677
}
6778
} catch (error) {
6879
errors.push(error);
@@ -76,8 +87,9 @@ function coerceVariableValues(
7687
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
7788
inputs: { readonly [variable: string]: unknown },
7889
onError: (error: GraphQLError) => void,
79-
): { [variable: string]: unknown } {
80-
const coercedValues: { [variable: string]: unknown } = {};
90+
): VariableValues {
91+
const sources: ObjMap<VariableValueSource> = Object.create(null);
92+
const coerced: ObjMap<unknown> = Object.create(null);
8193
for (const varDefNode of varDefNodes) {
8294
const varName = varDefNode.variable.name.value;
8395
const varType = typeFromAST(schema, varDefNode.type);
@@ -95,11 +107,14 @@ function coerceVariableValues(
95107
}
96108

97109
if (!Object.hasOwn(inputs, varName)) {
98-
if (varDefNode.defaultValue) {
99-
coercedValues[varName] = coerceInputLiteral(
100-
varDefNode.defaultValue,
101-
varType,
102-
);
110+
const defaultValue = varDefNode.defaultValue;
111+
if (defaultValue) {
112+
sources[varName] = {
113+
variable: varDefNode,
114+
type: varType,
115+
value: undefined,
116+
};
117+
coerced[varName] = coerceInputLiteral(defaultValue, varType);
103118
} else if (isNonNullType(varType)) {
104119
onError(
105120
new GraphQLError(
@@ -122,7 +137,8 @@ function coerceVariableValues(
122137
continue;
123138
}
124139

125-
coercedValues[varName] = coerceInputValue(
140+
sources[varName] = { variable: varDefNode, type: varType, value };
141+
coerced[varName] = coerceInputValue(
126142
value,
127143
varType,
128144
(path, invalidValue, error) => {
@@ -141,7 +157,7 @@ function coerceVariableValues(
141157
);
142158
}
143159

144-
return coercedValues;
160+
return { sources, coerced };
145161
}
146162

147163
/**
@@ -155,7 +171,7 @@ function coerceVariableValues(
155171
export function getArgumentValues(
156172
def: GraphQLField<unknown, unknown> | GraphQLDirective,
157173
node: FieldNode | DirectiveNode,
158-
variableValues?: Maybe<ObjMap<unknown>>,
174+
variableValues?: Maybe<VariableValues>,
159175
): { [argument: string]: unknown } {
160176
const coercedValues: { [argument: string]: unknown } = {};
161177

@@ -191,7 +207,7 @@ export function getArgumentValues(
191207
const variableName = valueNode.name.value;
192208
if (
193209
variableValues == null ||
194-
!Object.hasOwn(variableValues, variableName)
210+
!Object.hasOwn(variableValues.coerced, variableName)
195211
) {
196212
if (argDef.defaultValue) {
197213
coercedValues[name] = coerceDefaultValue(
@@ -207,7 +223,7 @@ export function getArgumentValues(
207223
}
208224
continue;
209225
}
210-
isNull = variableValues[variableName] == null;
226+
isNull = variableValues.coerced[variableName] == null;
211227
}
212228

213229
if (isNull && isNonNullType(argType)) {
@@ -248,7 +264,7 @@ export function getArgumentValues(
248264
export function getDirectiveValues(
249265
directiveDef: GraphQLDirective,
250266
node: { readonly directives?: ReadonlyArray<DirectiveNode> | undefined },
251-
variableValues?: Maybe<ObjMap<unknown>>,
267+
variableValues?: Maybe<VariableValues>,
252268
): undefined | { [argument: string]: unknown } {
253269
const directiveNode = node.directives?.find(
254270
(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)