Skip to content

Commit d177a87

Browse files
yaacovCRleebyron
andcommitted
Preserve sources of variable values (graphql#3811)
[graphql#3077 rebased on main](graphql#3077). Depends on graphql#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 graphql#3065. This PR is pulled out as a pre-req to aid review --------- Co-authored-by: Lee Byron <lee.byron@robinhood.com>
1 parent 3cd4015 commit d177a87

8 files changed

Lines changed: 218 additions & 106 deletions

File tree

src/execution/__tests__/executor-test.ts

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

src/execution/collectFields.ts

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,12 @@ import type { GraphQLSchema } from '../type/schema.js';
2121
import { typeFromAST } from '../utilities/typeFromAST.js';
2222

2323
import type { GraphQLVariableSignature } from './getVariableSignature.js';
24-
import { experimentalGetArgumentValues, getDirectiveValues } from './values.js';
25-
26-
export interface FragmentVariables {
27-
signatures: ObjMap<GraphQLVariableSignature>;
28-
values: ObjMap<unknown>;
29-
}
24+
import type { VariableValues } from './values.js';
25+
import { getDirectiveValues, getFragmentVariableValues } from './values.js';
3026

3127
export interface FieldDetails {
3228
node: FieldNode;
33-
fragmentVariables?: FragmentVariables | undefined;
29+
fragmentVariableValues?: VariableValues | undefined;
3430
}
3531

3632
export type FieldGroup = ReadonlyArray<FieldDetails>;
@@ -45,7 +41,7 @@ export interface FragmentDetails {
4541
interface CollectFieldsContext {
4642
schema: GraphQLSchema;
4743
fragments: ObjMap<FragmentDetails>;
48-
variableValues: { [variable: string]: unknown };
44+
variableValues: VariableValues;
4945
runtimeType: GraphQLObjectType;
5046
visitedFragmentNames: Set<string>;
5147
}
@@ -62,7 +58,7 @@ interface CollectFieldsContext {
6258
export function collectFields(
6359
schema: GraphQLSchema,
6460
fragments: ObjMap<FragmentDetails>,
65-
variableValues: { [variable: string]: unknown },
61+
variableValues: VariableValues,
6662
runtimeType: GraphQLObjectType,
6763
selectionSet: SelectionSetNode,
6864
): GroupedFieldSet {
@@ -92,7 +88,7 @@ export function collectFields(
9288
export function collectSubfields(
9389
schema: GraphQLSchema,
9490
fragments: ObjMap<FragmentDetails>,
95-
variableValues: { [variable: string]: unknown },
91+
variableValues: VariableValues,
9692
returnType: GraphQLObjectType,
9793
fieldGroup: FieldGroup,
9894
): GroupedFieldSet {
@@ -108,12 +104,12 @@ export function collectSubfields(
108104
for (const fieldDetail of fieldGroup) {
109105
const selectionSet = fieldDetail.node.selectionSet;
110106
if (selectionSet) {
111-
const { fragmentVariables } = fieldDetail;
107+
const { fragmentVariableValues } = fieldDetail;
112108
collectFieldsImpl(
113109
context,
114110
selectionSet,
115111
subGroupedFieldSet,
116-
fragmentVariables,
112+
fragmentVariableValues,
117113
);
118114
}
119115
}
@@ -125,7 +121,7 @@ function collectFieldsImpl(
125121
context: CollectFieldsContext,
126122
selectionSet: SelectionSetNode,
127123
groupedFieldSet: AccumulatorMap<string, FieldDetails>,
128-
fragmentVariables?: FragmentVariables,
124+
fragmentVariableValues?: VariableValues,
129125
): void {
130126
const {
131127
schema,
@@ -138,18 +134,24 @@ function collectFieldsImpl(
138134
for (const selection of selectionSet.selections) {
139135
switch (selection.kind) {
140136
case Kind.FIELD: {
141-
if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
137+
if (
138+
!shouldIncludeNode(selection, variableValues, fragmentVariableValues)
139+
) {
142140
continue;
143141
}
144142
groupedFieldSet.add(getFieldEntryKey(selection), {
145143
node: selection,
146-
fragmentVariables,
144+
fragmentVariableValues,
147145
});
148146
break;
149147
}
150148
case Kind.INLINE_FRAGMENT: {
151149
if (
152-
!shouldIncludeNode(selection, variableValues, fragmentVariables) ||
150+
!shouldIncludeNode(
151+
selection,
152+
variableValues,
153+
fragmentVariableValues,
154+
) ||
153155
!doesFragmentConditionMatch(schema, selection, runtimeType)
154156
) {
155157
continue;
@@ -159,16 +161,17 @@ function collectFieldsImpl(
159161
context,
160162
selection.selectionSet,
161163
groupedFieldSet,
162-
fragmentVariables,
164+
fragmentVariableValues,
163165
);
166+
164167
break;
165168
}
166169
case Kind.FRAGMENT_SPREAD: {
167170
const fragName = selection.name.value;
168171

169172
if (
170173
visitedFragmentNames.has(fragName) ||
171-
!shouldIncludeNode(selection, variableValues, fragmentVariables)
174+
!shouldIncludeNode(selection, variableValues, fragmentVariableValues)
172175
) {
173176
continue;
174177
}
@@ -182,25 +185,22 @@ function collectFieldsImpl(
182185
}
183186

184187
const fragmentVariableSignatures = fragment.variableSignatures;
185-
let newFragmentVariables: FragmentVariables | undefined;
188+
let newFragmentVariableValues: VariableValues | undefined;
186189
if (fragmentVariableSignatures) {
187-
newFragmentVariables = {
188-
signatures: fragmentVariableSignatures,
189-
values: experimentalGetArgumentValues(
190-
selection,
191-
Object.values(fragmentVariableSignatures),
192-
variableValues,
193-
fragmentVariables,
194-
),
195-
};
190+
newFragmentVariableValues = getFragmentVariableValues(
191+
selection,
192+
fragmentVariableSignatures,
193+
variableValues,
194+
fragmentVariableValues,
195+
);
196196
}
197197

198198
visitedFragmentNames.add(fragName);
199199
collectFieldsImpl(
200200
context,
201201
fragment.definition.selectionSet,
202202
groupedFieldSet,
203-
newFragmentVariables,
203+
newFragmentVariableValues,
204204
);
205205
break;
206206
}
@@ -214,14 +214,14 @@ function collectFieldsImpl(
214214
*/
215215
function shouldIncludeNode(
216216
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
217-
variableValues: { [variable: string]: unknown },
218-
fragmentVariables: FragmentVariables | undefined,
217+
variableValues: VariableValues,
218+
fragmentVariableValues: VariableValues | undefined,
219219
): boolean {
220220
const skip = getDirectiveValues(
221221
GraphQLSkipDirective,
222222
node,
223223
variableValues,
224-
fragmentVariables,
224+
fragmentVariableValues,
225225
);
226226
if (skip?.if === true) {
227227
return false;
@@ -231,7 +231,7 @@ function shouldIncludeNode(
231231
GraphQLIncludeDirective,
232232
node,
233233
variableValues,
234-
fragmentVariables,
234+
fragmentVariableValues,
235235
);
236236
if (include?.if === false) {
237237
return false;

src/execution/execute.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import {
5858
} from './collectFields.js';
5959
import { getVariableSignature } from './getVariableSignature.js';
6060
import { mapAsyncIterable } from './mapAsyncIterable.js';
61+
import type { VariableValues } from './values.js';
6162
import {
6263
experimentalGetArgumentValues,
6364
getArgumentValues,
@@ -120,7 +121,7 @@ export interface ExecutionContext {
120121
rootValue: unknown;
121122
contextValue: unknown;
122123
operation: OperationDefinitionNode;
123-
variableValues: { [variable: string]: unknown };
124+
variableValues: VariableValues;
124125
fieldResolver: GraphQLFieldResolver<any, any>;
125126
typeResolver: GraphQLTypeResolver<any, any>;
126127
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
@@ -354,15 +355,15 @@ export function buildExecutionContext(
354355
/* c8 ignore next */
355356
const variableDefinitions = operation.variableDefinitions ?? [];
356357

357-
const coercedVariableValues = getVariableValues(
358+
const variableValuesOrErrors = getVariableValues(
358359
schema,
359360
variableDefinitions,
360361
rawVariableValues ?? {},
361362
{ maxErrors: options?.maxCoercionErrors ?? 50 },
362363
);
363364

364-
if (coercedVariableValues.errors) {
365-
return coercedVariableValues.errors;
365+
if (variableValuesOrErrors.errors) {
366+
return variableValuesOrErrors.errors;
366367
}
367368

368369
return {
@@ -371,7 +372,7 @@ export function buildExecutionContext(
371372
rootValue,
372373
contextValue,
373374
operation,
374-
variableValues: coercedVariableValues.coerced,
375+
variableValues: variableValuesOrErrors.variableValues,
375376
fieldResolver: fieldResolver ?? defaultFieldResolver,
376377
typeResolver: typeResolver ?? defaultTypeResolver,
377378
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
@@ -557,7 +558,7 @@ function executeField(
557558
fieldGroup[0].node,
558559
fieldDef.args,
559560
exeContext.variableValues,
560-
fieldGroup[0].fragmentVariables,
561+
fieldGroup[0].fragmentVariableValues,
561562
);
562563

563564
// The resolve function's optional third argument is a context value that

0 commit comments

Comments
 (0)