Skip to content

Commit 9f85bc3

Browse files
leebyronyaacovCR
authored andcommitted
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 619c450 commit 9f85bc3

File tree

6 files changed

+143
-67
lines changed

6 files changed

+143
-67
lines changed

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 DeferUsage {
@@ -39,7 +40,7 @@ export interface FieldDetails {
3940
interface CollectFieldsContext {
4041
schema: GraphQLSchema;
4142
fragments: ObjMap<FragmentDefinitionNode>;
42-
variableValues: { [variable: string]: unknown };
43+
variableValues: VariableValues;
4344
operation: OperationDefinitionNode;
4445
runtimeType: GraphQLObjectType;
4546
visitedFragmentNames: Set<string>;
@@ -57,7 +58,7 @@ interface CollectFieldsContext {
5758
export function collectFields(
5859
schema: GraphQLSchema,
5960
fragments: ObjMap<FragmentDefinitionNode>,
60-
variableValues: { [variable: string]: unknown },
61+
variableValues: VariableValues,
6162
runtimeType: GraphQLObjectType,
6263
operation: OperationDefinitionNode,
6364
): Map<string, ReadonlyArray<FieldDetails>> {
@@ -89,7 +90,7 @@ export function collectFields(
8990
export function collectSubfields(
9091
schema: GraphQLSchema,
9192
fragments: ObjMap<FragmentDefinitionNode>,
92-
variableValues: { [variable: string]: unknown },
93+
variableValues: VariableValues,
9394
operation: OperationDefinitionNode,
9495
returnType: GraphQLObjectType,
9596
fieldDetails: ReadonlyArray<FieldDetails>,
@@ -221,7 +222,7 @@ function collectFieldsImpl(
221222
*/
222223
function getDeferUsage(
223224
operation: OperationDefinitionNode,
224-
variableValues: { [variable: string]: unknown },
225+
variableValues: VariableValues,
225226
node: FragmentSpreadNode | InlineFragmentNode,
226227
parentDeferUsage: DeferUsage | undefined,
227228
): DeferUsage | undefined {
@@ -251,7 +252,7 @@ function getDeferUsage(
251252
* directives, where `@skip` has higher precedence than `@include`.
252253
*/
253254
function shouldIncludeNode(
254-
variableValues: { [variable: string]: unknown },
255+
variableValues: VariableValues,
255256
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
256257
): boolean {
257258
const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);

src/execution/execute.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
StreamRecord,
7171
} from './IncrementalPublisher.js';
7272
import { mapAsyncIterable } from './mapAsyncIterable.js';
73+
import type { VariableValues } from './values.js';
7374
import {
7475
getArgumentValues,
7576
getDirectiveValues,
@@ -139,7 +140,7 @@ export interface ExecutionContext {
139140
rootValue: unknown;
140141
contextValue: unknown;
141142
operation: OperationDefinitionNode;
142-
variableValues: { [variable: string]: unknown };
143+
variableValues: VariableValues;
143144
fieldResolver: GraphQLFieldResolver<any, any>;
144145
typeResolver: GraphQLTypeResolver<any, any>;
145146
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
@@ -350,15 +351,15 @@ export function buildExecutionContext(
350351
/* c8 ignore next */
351352
const variableDefinitions = operation.variableDefinitions ?? [];
352353

353-
const coercedVariableValues = getVariableValues(
354+
const variableValuesOrErrors = getVariableValues(
354355
schema,
355356
variableDefinitions,
356357
rawVariableValues ?? {},
357358
{ maxErrors: 50 },
358359
);
359360

360-
if (coercedVariableValues.errors) {
361-
return coercedVariableValues.errors;
361+
if (variableValuesOrErrors.errors) {
362+
return variableValuesOrErrors.errors;
362363
}
363364

364365
return {
@@ -367,7 +368,7 @@ export function buildExecutionContext(
367368
rootValue,
368369
contextValue,
369370
operation,
370-
variableValues: coercedVariableValues.coerced,
371+
variableValues: variableValuesOrErrors.variableValues,
371372
fieldResolver: fieldResolver ?? defaultFieldResolver,
372373
typeResolver: typeResolver ?? defaultTypeResolver,
373374
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -714,7 +715,7 @@ export function buildResolveInfo(
714715
fragments: exeContext.fragments,
715716
rootValue: exeContext.rootValue,
716717
operation: exeContext.operation,
717-
variableValues: exeContext.variableValues,
718+
variableValues: exeContext.variableValues.coerced,
718719
};
719720
}
720721

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.js';
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,
@@ -556,20 +561,29 @@ describe('coerceInputLiteral', () => {
556561
valueText: string,
557562
type: GraphQLInputType,
558563
expected: unknown,
559-
variables?: ObjMap<unknown>,
564+
variableValues?: VariableValues,
560565
) {
561566
const ast = parseValue(valueText);
562-
const value = coerceInputLiteral(ast, type, variables);
567+
const value = coerceInputLiteral(ast, type, variableValues);
563568
expect(value).to.deep.equal(expected);
564569
}
565570

566571
function testWithVariables(
567-
variables: ObjMap<unknown>,
572+
variableDefs: string,
573+
inputs: ReadOnlyObjMap<unknown>,
568574
valueText: string,
569575
type: GraphQLInputType,
570576
expected: unknown,
571577
) {
572-
test(valueText, type, expected, variables);
578+
const parser = new Parser(variableDefs);
579+
parser.expectToken(TokenKind.SOF);
580+
const variableValuesOrErrors = getVariableValues(
581+
new GraphQLSchema({}),
582+
parser.parseVariableDefinitions(),
583+
inputs,
584+
);
585+
invariant(variableValuesOrErrors.variableValues !== undefined);
586+
test(valueText, type, expected, variableValuesOrErrors.variableValues);
573587
}
574588

575589
it('converts according to input coercion rules', () => {
@@ -788,19 +802,55 @@ describe('coerceInputLiteral', () => {
788802

789803
it('accepts variable values assuming already coerced', () => {
790804
test('$var', GraphQLBoolean, undefined);
791-
testWithVariables({ var: true }, '$var', GraphQLBoolean, true);
792-
testWithVariables({ var: null }, '$var', GraphQLBoolean, null);
793-
testWithVariables({ var: null }, '$var', nonNullBool, undefined);
805+
testWithVariables(
806+
'($var: Boolean)',
807+
{ var: true },
808+
'$var',
809+
GraphQLBoolean,
810+
true,
811+
);
812+
testWithVariables(
813+
'($var: Boolean)',
814+
{ var: null },
815+
'$var',
816+
GraphQLBoolean,
817+
null,
818+
);
819+
testWithVariables(
820+
'($var: Boolean)',
821+
{ var: null },
822+
'$var',
823+
nonNullBool,
824+
undefined,
825+
);
794826
});
795827

796828
it('asserts variables are provided as items in lists', () => {
797829
test('[ $foo ]', listOfBool, [null]);
798830
test('[ $foo ]', listOfNonNullBool, undefined);
799-
testWithVariables({ foo: true }, '[ $foo ]', listOfNonNullBool, [true]);
831+
testWithVariables(
832+
'($foo: Boolean)',
833+
{ foo: true },
834+
'[ $foo ]',
835+
listOfNonNullBool,
836+
[true],
837+
);
800838
// Note: variables are expected to have already been coerced, so we
801839
// do not expect the singleton wrapping behavior for variables.
802-
testWithVariables({ foo: true }, '$foo', listOfNonNullBool, true);
803-
testWithVariables({ foo: [true] }, '$foo', listOfNonNullBool, [true]);
840+
testWithVariables(
841+
'($foo: Boolean)',
842+
{ foo: true },
843+
'$foo',
844+
listOfNonNullBool,
845+
true,
846+
);
847+
testWithVariables(
848+
'($foo: [Boolean])',
849+
{ foo: [true] },
850+
'$foo',
851+
listOfNonNullBool,
852+
[true],
853+
);
804854
});
805855

806856
it('omits input object fields for unprovided variables', () => {
@@ -809,10 +859,13 @@ describe('coerceInputLiteral', () => {
809859
requiredBool: true,
810860
});
811861
test('{ requiredBool: $foo }', testInputObj, undefined);
812-
testWithVariables({ foo: true }, '{ requiredBool: $foo }', testInputObj, {
813-
int: 42,
814-
requiredBool: true,
815-
});
862+
testWithVariables(
863+
'($foo: Boolean)',
864+
{ foo: true },
865+
'{ requiredBool: $foo }',
866+
testInputObj,
867+
{ int: 42, requiredBool: true },
868+
);
816869
});
817870
});
818871

0 commit comments

Comments
 (0)