Skip to content

Commit b83db68

Browse files
yaacovCRyaacovCR
committed
execution suggestions
1 parent 1708107 commit b83db68

File tree

7 files changed

+237
-226
lines changed

7 files changed

+237
-226
lines changed

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

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
import { GraphQLBoolean, GraphQLString } from '../../type/scalars.js';
3131
import { GraphQLSchema } from '../../type/schema.js';
3232

33-
import { executeSync } from '../execute.js';
33+
import { executeSync, experimentalExecuteIncrementally } from '../execute.js';
3434
import { getVariableValues } from '../values.js';
3535

3636
const TestFaultyScalarGraphQLError = new GraphQLError(
@@ -1499,30 +1499,23 @@ describe('Execute: Handles inputs', () => {
14991499
});
15001500

15011501
it('when a nullable argument to a directive with a field default is not provided and shadowed by an operation variable', () => {
1502-
const result = executeQueryWithFragmentArguments(`
1503-
query($value: Boolean) {
1504-
...a
1505-
}
1506-
fragment a($value: Boolean) on TestType {
1507-
fieldWithNonNullableStringInput @skip(if: $value)
1508-
}
1509-
`);
1502+
// this test uses the @defer directive and incremental delivery because the `if` argument for skip/include have no field defaults
1503+
const document = parse(
1504+
`
1505+
query($shouldDefer: Boolean = false) {
1506+
...a
1507+
}
1508+
fragment a($shouldDefer: Boolean) on TestType {
1509+
... @defer(if: $shouldDefer) {
1510+
fieldWithDefaultArgumentValue
1511+
}
1512+
}
1513+
`,
1514+
{ experimentalFragmentArguments: true },
1515+
);
1516+
const result = experimentalExecuteIncrementally({ schema, document });
15101517

1511-
expectJSON(result).toDeepEqual({
1512-
data: null,
1513-
errors: [
1514-
{
1515-
locations: [
1516-
{
1517-
column: 53,
1518-
line: 6,
1519-
},
1520-
],
1521-
message:
1522-
'Argument "if" of non-null type "Boolean!" must not be null.',
1523-
},
1524-
],
1525-
});
1518+
expect(result).to.include.keys('initialResult', 'subsequentResults');
15261519
});
15271520
});
15281521
});

‎src/execution/collectFields.ts

Lines changed: 69 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -22,33 +22,44 @@ import {
2222
} from '../type/directives.js';
2323
import type { GraphQLSchema } from '../type/schema.js';
2424

25+
import type { GraphQLVariableSignature } from '../utilities/getVariableSignature.js';
2526
import { typeFromAST } from '../utilities/typeFromAST.js';
2627

27-
import { getArgumentValuesFromSpread, getDirectiveValues } from './values.js';
28+
import { experimentalGetArgumentValues, getDirectiveValues } from './values.js';
2829

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

35+
export interface FragmentVariables {
36+
signatures: ObjMap<GraphQLVariableSignature>;
37+
values: ObjMap<unknown>;
38+
}
39+
3440
export interface FieldDetails {
3541
node: FieldNode;
36-
deferUsage: DeferUsage | undefined;
37-
fragmentVariableValues?: ObjMap<unknown> | undefined;
42+
deferUsage?: DeferUsage | undefined;
43+
fragmentVariables?: FragmentVariables | undefined;
3844
}
3945

4046
export type FieldGroup = ReadonlyArray<FieldDetails>;
4147

4248
export type GroupedFieldSet = ReadonlyMap<string, FieldGroup>;
4349

50+
export interface FragmentDetails {
51+
definition: FragmentDefinitionNode;
52+
variableSignatures?: ObjMap<GraphQLVariableSignature> | undefined;
53+
}
54+
4455
interface CollectFieldsContext {
4556
schema: GraphQLSchema;
46-
fragments: ObjMap<FragmentDefinitionNode>;
57+
fragments: ObjMap<FragmentDetails>;
58+
variableValues: { [variable: string]: unknown };
59+
fragmentVariableValues?: FragmentVariables;
4760
operation: OperationDefinitionNode;
4861
runtimeType: GraphQLObjectType;
4962
visitedFragmentNames: Set<string>;
50-
localVariableValues: { [variable: string]: unknown } | undefined;
51-
variableValues: { [variable: string]: unknown };
5263
}
5364

5465
/**
@@ -62,7 +73,7 @@ interface CollectFieldsContext {
6273
*/
6374
export function collectFields(
6475
schema: GraphQLSchema,
65-
fragments: ObjMap<FragmentDefinitionNode>,
76+
fragments: ObjMap<FragmentDetails>,
6677
variableValues: { [variable: string]: unknown },
6778
runtimeType: GraphQLObjectType,
6879
operation: OperationDefinitionNode,
@@ -75,10 +86,9 @@ export function collectFields(
7586
const context: CollectFieldsContext = {
7687
schema,
7788
fragments,
78-
runtimeType,
7989
variableValues,
90+
runtimeType,
8091
operation,
81-
localVariableValues: undefined,
8292
visitedFragmentNames: new Set(),
8393
};
8494

@@ -104,7 +114,7 @@ export function collectFields(
104114
// eslint-disable-next-line max-params
105115
export function collectSubfields(
106116
schema: GraphQLSchema,
107-
fragments: ObjMap<FragmentDefinitionNode>,
117+
fragments: ObjMap<FragmentDetails>,
108118
variableValues: { [variable: string]: unknown },
109119
operation: OperationDefinitionNode,
110120
returnType: GraphQLObjectType,
@@ -116,9 +126,8 @@ export function collectSubfields(
116126
const context: CollectFieldsContext = {
117127
schema,
118128
fragments,
119-
runtimeType: returnType,
120-
localVariableValues: undefined,
121129
variableValues,
130+
runtimeType: returnType,
122131
operation,
123132
visitedFragmentNames: new Set(),
124133
};
@@ -144,47 +153,40 @@ export function collectSubfields(
144153
};
145154
}
146155

156+
// eslint-disable-next-line max-params
147157
function collectFieldsImpl(
148158
context: CollectFieldsContext,
149159
selectionSet: SelectionSetNode,
150160
groupedFieldSet: AccumulatorMap<string, FieldDetails>,
151161
newDeferUsages: Array<DeferUsage>,
152162
deferUsage?: DeferUsage,
163+
fragmentVariables?: FragmentVariables,
153164
): void {
154165
const {
155166
schema,
156167
fragments,
157-
runtimeType,
158168
variableValues,
159-
localVariableValues,
169+
runtimeType,
160170
operation,
161171
visitedFragmentNames,
162172
} = context;
163173

164174
for (const selection of selectionSet.selections) {
165175
switch (selection.kind) {
166176
case Kind.FIELD: {
167-
if (
168-
!shouldIncludeNode(
169-
{ ...variableValues, ...localVariableValues },
170-
selection,
171-
)
172-
) {
177+
if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
173178
continue;
174179
}
175180
groupedFieldSet.add(getFieldEntryKey(selection), {
176181
node: selection,
177182
deferUsage,
178-
fragmentVariableValues: localVariableValues ?? undefined,
183+
fragmentVariables,
179184
});
180185
break;
181186
}
182187
case Kind.INLINE_FRAGMENT: {
183188
if (
184-
!shouldIncludeNode(
185-
{ ...variableValues, ...localVariableValues },
186-
selection,
187-
) ||
189+
!shouldIncludeNode(selection, variableValues, fragmentVariables) ||
188190
!doesFragmentConditionMatch(schema, selection, runtimeType)
189191
) {
190192
continue;
@@ -193,6 +195,7 @@ function collectFieldsImpl(
193195
const newDeferUsage = getDeferUsage(
194196
operation,
195197
variableValues,
198+
fragmentVariables,
196199
selection,
197200
deferUsage,
198201
);
@@ -204,6 +207,7 @@ function collectFieldsImpl(
204207
groupedFieldSet,
205208
newDeferUsages,
206209
deferUsage,
210+
fragmentVariables,
207211
);
208212
} else {
209213
newDeferUsages.push(newDeferUsage);
@@ -213,76 +217,72 @@ function collectFieldsImpl(
213217
groupedFieldSet,
214218
newDeferUsages,
215219
newDeferUsage,
220+
fragmentVariables,
216221
);
217222
}
218223

219224
break;
220225
}
221226
case Kind.FRAGMENT_SPREAD: {
222-
const fragmentName = selection.name.value;
227+
const fragName = selection.name.value;
223228

224229
const newDeferUsage = getDeferUsage(
225230
operation,
226-
{ ...variableValues, ...localVariableValues },
231+
variableValues,
232+
fragmentVariables,
227233
selection,
228234
deferUsage,
229235
);
230236

231237
if (
232238
!newDeferUsage &&
233-
(visitedFragmentNames.has(fragmentName) ||
234-
!shouldIncludeNode(
235-
{ ...variableValues, ...localVariableValues },
236-
selection,
237-
))
239+
(visitedFragmentNames.has(fragName) ||
240+
!shouldIncludeNode(selection, variableValues, fragmentVariables))
238241
) {
239242
continue;
240243
}
241244

242-
const fragment = fragments[fragmentName];
245+
const fragment = fragments[fragName];
243246
if (
244247
fragment == null ||
245-
!doesFragmentConditionMatch(schema, fragment, runtimeType)
248+
!doesFragmentConditionMatch(schema, fragment.definition, runtimeType)
246249
) {
247250
continue;
248251
}
249252

250-
// We need to introduce a concept of shadowing:
251-
//
252-
// - when a fragment defines a variable that is in the parent scope but not given
253-
// in the fragment-spread we need to look at this variable as undefined and check
254-
// whether the definition has a defaultValue, if not remove it from the variableValues.
255-
// - when a fragment does not define a variable we need to copy it over from the parent
256-
// scope as that variable can still get used in spreads later on in the selectionSet.
257-
// - when a value is passed in through the fragment-spread we need to copy over the key-value
258-
// into our variable-values.
259-
context.localVariableValues = fragment.variableDefinitions
260-
? getArgumentValuesFromSpread(
253+
const fragmentVariableSignatures = fragment.variableSignatures;
254+
let newFragmentVariables: FragmentVariables | undefined;
255+
if (fragmentVariableSignatures) {
256+
newFragmentVariables = {
257+
signatures: fragmentVariableSignatures,
258+
values: experimentalGetArgumentValues(
261259
selection,
262-
schema,
263-
fragment.variableDefinitions,
260+
Object.values(fragmentVariableSignatures),
264261
variableValues,
265-
context.localVariableValues,
266-
)
267-
: undefined;
262+
fragmentVariables,
263+
),
264+
};
265+
}
268266

269267
if (!newDeferUsage) {
270-
visitedFragmentNames.add(fragmentName);
268+
visitedFragmentNames.add(fragName);
271269
collectFieldsImpl(
272270
context,
273-
fragment.selectionSet,
271+
fragment.definition.selectionSet,
274272
groupedFieldSet,
275273
newDeferUsages,
276274
deferUsage,
275+
newFragmentVariables,
277276
);
278277
} else {
279278
newDeferUsages.push(newDeferUsage);
280279
collectFieldsImpl(
281280
context,
282-
fragment.selectionSet,
281+
fragment.definition.selectionSet,
283282
groupedFieldSet,
284283
newDeferUsages,
285284
newDeferUsage,
285+
newFragmentVariables,
286286
);
287287
}
288288
break;
@@ -299,10 +299,16 @@ function collectFieldsImpl(
299299
function getDeferUsage(
300300
operation: OperationDefinitionNode,
301301
variableValues: { [variable: string]: unknown },
302+
fragmentVariables: FragmentVariables | undefined,
302303
node: FragmentSpreadNode | InlineFragmentNode,
303304
parentDeferUsage: DeferUsage | undefined,
304305
): DeferUsage | undefined {
305-
const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues);
306+
const defer = getDirectiveValues(
307+
GraphQLDeferDirective,
308+
node,
309+
variableValues,
310+
fragmentVariables,
311+
);
306312

307313
if (!defer) {
308314
return;
@@ -328,10 +334,16 @@ function getDeferUsage(
328334
* directives, where `@skip` has higher precedence than `@include`.
329335
*/
330336
function shouldIncludeNode(
331-
variableValues: { [variable: string]: unknown },
332337
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
338+
variableValues: { [variable: string]: unknown },
339+
fragmentVariables: FragmentVariables | undefined,
333340
): boolean {
334-
const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);
341+
const skip = getDirectiveValues(
342+
GraphQLSkipDirective,
343+
node,
344+
variableValues,
345+
fragmentVariables,
346+
);
335347
if (skip?.if === true) {
336348
return false;
337349
}
@@ -340,6 +352,7 @@ function shouldIncludeNode(
340352
GraphQLIncludeDirective,
341353
node,
342354
variableValues,
355+
fragmentVariables,
343356
);
344357
if (include?.if === false) {
345358
return false;

0 commit comments

Comments
 (0)