Skip to content

Commit 7dd825e

Browse files
yaacovCRleebyron
yaacovCR
and
leebyron
authored
Preserve sources of variable values (#3811)
[#3077 rebased on main](#3077). Depends on #3810 @leebyron comments from original PR: > 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`. > > While variable sources are not used directly here, they're used directly in #3065. This PR is pulled out as a pre-req to aid review --------- Co-authored-by: Lee Byron <lee.byron@robinhood.com>
1 parent 992b768 commit 7dd825e

File tree

8 files changed

+230
-115
lines changed

8 files changed

+230
-115
lines changed

‎src/execution/__tests__/executor-test.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,19 @@ describe('Execute: Handles basic execution tasks', () => {
240240
expect(resolvedInfo).to.deep.include({
241241
fieldNodes: [field],
242242
path: { prev: undefined, key: 'result', typename: 'Test' },
243-
variableValues: { var: 'abc' },
243+
variableValues: {
244+
sources: {
245+
var: {
246+
signature: {
247+
name: 'var',
248+
type: GraphQLString,
249+
defaultValue: undefined,
250+
},
251+
value: 'abc',
252+
},
253+
},
254+
coerced: { var: 'abc' },
255+
},
244256
});
245257
});
246258

‎src/execution/collectFields.ts‎

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,18 @@ import type { GraphQLSchema } from '../type/schema.js';
2525
import { typeFromAST } from '../utilities/typeFromAST.js';
2626

2727
import type { GraphQLVariableSignature } from './getVariableSignature.js';
28-
import { experimentalGetArgumentValues, getDirectiveValues } from './values.js';
28+
import type { VariableValues } from './values.js';
29+
import { getDirectiveValues, getFragmentVariableValues } from './values.js';
2930

3031
export interface DeferUsage {
3132
label: string | undefined;
3233
parentDeferUsage: DeferUsage | undefined;
3334
}
3435

35-
export interface FragmentVariables {
36-
signatures: ObjMap<GraphQLVariableSignature>;
37-
values: ObjMap<unknown>;
38-
}
39-
4036
export interface FieldDetails {
4137
node: FieldNode;
4238
deferUsage?: DeferUsage | undefined;
43-
fragmentVariables?: FragmentVariables | undefined;
39+
fragmentVariableValues?: VariableValues | undefined;
4440
}
4541

4642
export type FieldGroup = ReadonlyArray<FieldDetails>;
@@ -55,7 +51,7 @@ export interface FragmentDetails {
5551
interface CollectFieldsContext {
5652
schema: GraphQLSchema;
5753
fragments: ObjMap<FragmentDetails>;
58-
variableValues: { [variable: string]: unknown };
54+
variableValues: VariableValues;
5955
operation: OperationDefinitionNode;
6056
runtimeType: GraphQLObjectType;
6157
visitedFragmentNames: Set<string>;
@@ -73,7 +69,7 @@ interface CollectFieldsContext {
7369
export function collectFields(
7470
schema: GraphQLSchema,
7571
fragments: ObjMap<FragmentDetails>,
76-
variableValues: { [variable: string]: unknown },
72+
variableValues: VariableValues,
7773
runtimeType: GraphQLObjectType,
7874
operation: OperationDefinitionNode,
7975
): {
@@ -114,7 +110,7 @@ export function collectFields(
114110
export function collectSubfields(
115111
schema: GraphQLSchema,
116112
fragments: ObjMap<FragmentDetails>,
117-
variableValues: { [variable: string]: unknown },
113+
variableValues: VariableValues,
118114
operation: OperationDefinitionNode,
119115
returnType: GraphQLObjectType,
120116
fieldGroup: FieldGroup,
@@ -136,14 +132,14 @@ export function collectSubfields(
136132
for (const fieldDetail of fieldGroup) {
137133
const selectionSet = fieldDetail.node.selectionSet;
138134
if (selectionSet) {
139-
const { deferUsage, fragmentVariables } = fieldDetail;
135+
const { deferUsage, fragmentVariableValues } = fieldDetail;
140136
collectFieldsImpl(
141137
context,
142138
selectionSet,
143139
subGroupedFieldSet,
144140
newDeferUsages,
145141
deferUsage,
146-
fragmentVariables,
142+
fragmentVariableValues,
147143
);
148144
}
149145
}
@@ -161,7 +157,7 @@ function collectFieldsImpl(
161157
groupedFieldSet: AccumulatorMap<string, FieldDetails>,
162158
newDeferUsages: Array<DeferUsage>,
163159
deferUsage?: DeferUsage,
164-
fragmentVariables?: FragmentVariables,
160+
fragmentVariableValues?: VariableValues,
165161
): void {
166162
const {
167163
schema,
@@ -175,19 +171,25 @@ function collectFieldsImpl(
175171
for (const selection of selectionSet.selections) {
176172
switch (selection.kind) {
177173
case Kind.FIELD: {
178-
if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
174+
if (
175+
!shouldIncludeNode(selection, variableValues, fragmentVariableValues)
176+
) {
179177
continue;
180178
}
181179
groupedFieldSet.add(getFieldEntryKey(selection), {
182180
node: selection,
183181
deferUsage,
184-
fragmentVariables,
182+
fragmentVariableValues,
185183
});
186184
break;
187185
}
188186
case Kind.INLINE_FRAGMENT: {
189187
if (
190-
!shouldIncludeNode(selection, variableValues, fragmentVariables) ||
188+
!shouldIncludeNode(
189+
selection,
190+
variableValues,
191+
fragmentVariableValues,
192+
) ||
191193
!doesFragmentConditionMatch(schema, selection, runtimeType)
192194
) {
193195
continue;
@@ -196,7 +198,7 @@ function collectFieldsImpl(
196198
const newDeferUsage = getDeferUsage(
197199
operation,
198200
variableValues,
199-
fragmentVariables,
201+
fragmentVariableValues,
200202
selection,
201203
deferUsage,
202204
);
@@ -208,7 +210,7 @@ function collectFieldsImpl(
208210
groupedFieldSet,
209211
newDeferUsages,
210212
deferUsage,
211-
fragmentVariables,
213+
fragmentVariableValues,
212214
);
213215
} else {
214216
newDeferUsages.push(newDeferUsage);
@@ -218,7 +220,7 @@ function collectFieldsImpl(
218220
groupedFieldSet,
219221
newDeferUsages,
220222
newDeferUsage,
221-
fragmentVariables,
223+
fragmentVariableValues,
222224
);
223225
}
224226

@@ -230,15 +232,19 @@ function collectFieldsImpl(
230232
const newDeferUsage = getDeferUsage(
231233
operation,
232234
variableValues,
233-
fragmentVariables,
235+
fragmentVariableValues,
234236
selection,
235237
deferUsage,
236238
);
237239

238240
if (
239241
!newDeferUsage &&
240242
(visitedFragmentNames.has(fragName) ||
241-
!shouldIncludeNode(selection, variableValues, fragmentVariables))
243+
!shouldIncludeNode(
244+
selection,
245+
variableValues,
246+
fragmentVariableValues,
247+
))
242248
) {
243249
continue;
244250
}
@@ -252,17 +258,14 @@ function collectFieldsImpl(
252258
}
253259

254260
const fragmentVariableSignatures = fragment.variableSignatures;
255-
let newFragmentVariables: FragmentVariables | undefined;
261+
let newFragmentVariableValues: VariableValues | undefined;
256262
if (fragmentVariableSignatures) {
257-
newFragmentVariables = {
258-
signatures: fragmentVariableSignatures,
259-
values: experimentalGetArgumentValues(
260-
selection,
261-
Object.values(fragmentVariableSignatures),
262-
variableValues,
263-
fragmentVariables,
264-
),
265-
};
263+
newFragmentVariableValues = getFragmentVariableValues(
264+
selection,
265+
fragmentVariableSignatures,
266+
variableValues,
267+
fragmentVariableValues,
268+
);
266269
}
267270

268271
if (!newDeferUsage) {
@@ -273,7 +276,7 @@ function collectFieldsImpl(
273276
groupedFieldSet,
274277
newDeferUsages,
275278
deferUsage,
276-
newFragmentVariables,
279+
newFragmentVariableValues,
277280
);
278281
} else {
279282
newDeferUsages.push(newDeferUsage);
@@ -283,7 +286,7 @@ function collectFieldsImpl(
283286
groupedFieldSet,
284287
newDeferUsages,
285288
newDeferUsage,
286-
newFragmentVariables,
289+
newFragmentVariableValues,
287290
);
288291
}
289292
break;
@@ -299,16 +302,16 @@ function collectFieldsImpl(
299302
*/
300303
function getDeferUsage(
301304
operation: OperationDefinitionNode,
302-
variableValues: { [variable: string]: unknown },
303-
fragmentVariables: FragmentVariables | undefined,
305+
variableValues: VariableValues,
306+
fragmentVariableValues: VariableValues | undefined,
304307
node: FragmentSpreadNode | InlineFragmentNode,
305308
parentDeferUsage: DeferUsage | undefined,
306309
): DeferUsage | undefined {
307310
const defer = getDirectiveValues(
308311
GraphQLDeferDirective,
309312
node,
310313
variableValues,
311-
fragmentVariables,
314+
fragmentVariableValues,
312315
);
313316

314317
if (!defer) {
@@ -336,14 +339,14 @@ function getDeferUsage(
336339
*/
337340
function shouldIncludeNode(
338341
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
339-
variableValues: { [variable: string]: unknown },
340-
fragmentVariables: FragmentVariables | undefined,
342+
variableValues: VariableValues,
343+
fragmentVariableValues: VariableValues | undefined,
341344
): boolean {
342345
const skip = getDirectiveValues(
343346
GraphQLSkipDirective,
344347
node,
345348
variableValues,
346-
fragmentVariables,
349+
fragmentVariableValues,
347350
);
348351
if (skip?.if === true) {
349352
return false;
@@ -353,7 +356,7 @@ function shouldIncludeNode(
353356
GraphQLIncludeDirective,
354357
node,
355358
variableValues,
356-
fragmentVariables,
359+
fragmentVariableValues,
357360
);
358361
if (include?.if === false) {
359362
return false;

‎src/execution/execute.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import type {
7575
StreamRecord,
7676
} from './types.js';
7777
import { DeferredFragmentRecord } from './types.js';
78+
import type { VariableValues } from './values.js';
7879
import {
7980
experimentalGetArgumentValues,
8081
getArgumentValues,
@@ -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>;
@@ -510,15 +511,15 @@ export function buildExecutionContext(
510511
/* c8 ignore next */
511512
const variableDefinitions = operation.variableDefinitions ?? [];
512513

513-
const coercedVariableValues = getVariableValues(
514+
const variableValuesOrErrors = getVariableValues(
514515
schema,
515516
variableDefinitions,
516517
rawVariableValues ?? {},
517518
{ maxErrors: 50 },
518519
);
519520

520-
if (coercedVariableValues.errors) {
521-
return coercedVariableValues.errors;
521+
if (variableValuesOrErrors.errors) {
522+
return variableValuesOrErrors.errors;
522523
}
523524

524525
return {
@@ -527,7 +528,7 @@ export function buildExecutionContext(
527528
rootValue,
528529
contextValue,
529530
operation,
530-
variableValues: coercedVariableValues.coerced,
531+
variableValues: variableValuesOrErrors.variableValues,
531532
fieldResolver: fieldResolver ?? defaultFieldResolver,
532533
typeResolver: typeResolver ?? defaultTypeResolver,
533534
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -753,7 +754,7 @@ function executeField(
753754
fieldGroup[0].node,
754755
fieldDef.args,
755756
exeContext.variableValues,
756-
fieldGroup[0].fragmentVariables,
757+
fieldGroup[0].fragmentVariableValues,
757758
);
758759

759760
// The resolve function's optional third argument is a context value that
@@ -1062,7 +1063,7 @@ function getStreamUsage(
10621063
GraphQLStreamDirective,
10631064
fieldGroup[0].node,
10641065
exeContext.variableValues,
1065-
fieldGroup[0].fragmentVariables,
1066+
fieldGroup[0].fragmentVariableValues,
10661067
);
10671068

10681069
if (!stream) {
@@ -1091,7 +1092,7 @@ function getStreamUsage(
10911092
const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({
10921093
node: fieldDetails.node,
10931094
deferUsage: undefined,
1094-
fragmentVariables: fieldDetails.fragmentVariables,
1095+
fragmentVariablesValues: fieldDetails.fragmentVariableValues,
10951096
}));
10961097

10971098
const streamUsage = {

0 commit comments

Comments
 (0)