Skip to content

Commit 065c385

Browse files
JoviDeCroockyaacovCR
JoviDeCroock
and
yaacovCR
committed
Apply changes from v17 branch
Co-authored-by: Yaacov Rydzinski <[email protected]>
1 parent 97b9a8f commit 065c385

33 files changed

+936
-577
lines changed

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

+67-2
Original file line numberDiff line numberDiff line change
@@ -1201,7 +1201,7 @@ describe('Execute: Handles inputs', () => {
12011201

12021202
expect(result).to.have.property('errors');
12031203
expect(result.errors).to.have.length(1);
1204-
expect(result.errors?.[0]?.message).to.match(
1204+
expect(result.errors?.[0].message).to.match(
12051205
/Argument "value" of required type "String!"/,
12061206
);
12071207
});
@@ -1269,7 +1269,7 @@ describe('Execute: Handles inputs', () => {
12691269

12701270
expect(result).to.have.property('errors');
12711271
expect(result.errors).to.have.length(1);
1272-
expect(result.errors?.[0]?.message).to.match(
1272+
expect(result.errors?.[0].message).to.match(
12731273
/Argument "value" of non-null type "String!"/,
12741274
);
12751275
});
@@ -1307,6 +1307,22 @@ describe('Execute: Handles inputs', () => {
13071307
});
13081308
});
13091309

1310+
it('when a nullable argument without a field default is not provided and shadowed by an operation variable', () => {
1311+
const result = executeQueryWithFragmentArguments(`
1312+
query($x: String = "A") {
1313+
...a
1314+
}
1315+
fragment a($x: String) on TestType {
1316+
fieldWithNullableStringInput(input: $x)
1317+
}
1318+
`);
1319+
expect(result).to.deep.equal({
1320+
data: {
1321+
fieldWithNullableStringInput: null,
1322+
},
1323+
});
1324+
});
1325+
13101326
it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
13111327
const result = executeQueryWithFragmentArguments(`
13121328
query($x: String = "A") {
@@ -1411,6 +1427,27 @@ describe('Execute: Handles inputs', () => {
14111427
});
14121428
});
14131429

1430+
it('when argument variables with the same name are used directly and recursively', () => {
1431+
const result = executeQueryWithFragmentArguments(`
1432+
query {
1433+
...a(value: "A")
1434+
}
1435+
fragment a($value: String!) on TestType {
1436+
...b(value: "B")
1437+
fieldInFragmentA: fieldWithNonNullableStringInput(input: $value)
1438+
}
1439+
fragment b($value: String!) on TestType {
1440+
fieldInFragmentB: fieldWithNonNullableStringInput(input: $value)
1441+
}
1442+
`);
1443+
expect(result).to.deep.equal({
1444+
data: {
1445+
fieldInFragmentA: '"A"',
1446+
fieldInFragmentB: '"B"',
1447+
},
1448+
});
1449+
});
1450+
14141451
it('when argument passed in as list', () => {
14151452
const result = executeQueryWithFragmentArguments(`
14161453
query Q($opValue: String = "op") {
@@ -1433,5 +1470,33 @@ describe('Execute: Handles inputs', () => {
14331470
},
14341471
});
14351472
});
1473+
1474+
it('when argument passed to a directive', () => {
1475+
const result = executeQueryWithFragmentArguments(`
1476+
query {
1477+
...a(value: true)
1478+
}
1479+
fragment a($value: Boolean!) on TestType {
1480+
fieldWithNonNullableStringInput @skip(if: $value)
1481+
}
1482+
`);
1483+
expect(result).to.deep.equal({
1484+
data: {},
1485+
});
1486+
});
1487+
1488+
it('when argument passed to a directive on a nested field', () => {
1489+
const result = executeQueryWithFragmentArguments(`
1490+
query {
1491+
...a(value: true)
1492+
}
1493+
fragment a($value: Boolean!) on TestType {
1494+
nested { echo(input: "echo") @skip(if: $value) }
1495+
}
1496+
`);
1497+
expect(result).to.deep.equal({
1498+
data: { nested: {} },
1499+
});
1500+
});
14361501
});
14371502
});

‎src/execution/collectFields.ts

+47-33
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,24 @@ import {
1717
} from '../type/directives';
1818
import type { GraphQLSchema } from '../type/schema';
1919

20+
import type { GraphQLVariableSignature } from '../utilities/getVariableSignature';
2021
import { typeFromAST } from '../utilities/typeFromAST';
2122

22-
import { getArgumentValuesFromSpread, getDirectiveValues } from './values';
23+
import { experimentalGetArgumentValues, getDirectiveValues } from './values';
24+
25+
export interface FragmentVariables {
26+
signatures: ObjMap<GraphQLVariableSignature>;
27+
values: ObjMap<unknown>;
28+
}
2329

2430
export interface FieldDetails {
2531
node: FieldNode;
26-
fragmentVariableValues?: ObjMap<unknown> | undefined;
32+
fragmentVariables?: FragmentVariables | undefined;
33+
}
34+
35+
export interface FragmentDetails {
36+
definition: FragmentDefinitionNode;
37+
variableSignatures?: ObjMap<GraphQLVariableSignature> | undefined;
2738
}
2839

2940
/**
@@ -37,7 +48,7 @@ export interface FieldDetails {
3748
*/
3849
export function collectFields(
3950
schema: GraphQLSchema,
40-
fragments: ObjMap<FragmentDefinitionNode>,
51+
fragments: ObjMap<FragmentDetails>,
4152
variableValues: { [variable: string]: unknown },
4253
runtimeType: GraphQLObjectType,
4354
selectionSet: SelectionSetNode,
@@ -68,7 +79,7 @@ export function collectFields(
6879
*/
6980
export function collectSubfields(
7081
schema: GraphQLSchema,
71-
fragments: ObjMap<FragmentDefinitionNode>,
82+
fragments: ObjMap<FragmentDetails>,
7283
variableValues: { [variable: string]: unknown },
7384
returnType: GraphQLObjectType,
7485
fieldEntries: ReadonlyArray<FieldDetails>,
@@ -85,6 +96,7 @@ export function collectSubfields(
8596
entry.node.selectionSet,
8697
subFieldEntries,
8798
visitedFragmentNames,
99+
entry.fragmentVariables,
88100
);
89101
}
90102
}
@@ -93,41 +105,40 @@ export function collectSubfields(
93105

94106
function collectFieldsImpl(
95107
schema: GraphQLSchema,
96-
fragments: ObjMap<FragmentDefinitionNode>,
108+
fragments: ObjMap<FragmentDetails>,
97109
variableValues: { [variable: string]: unknown },
98110
runtimeType: GraphQLObjectType,
99111
selectionSet: SelectionSetNode,
100112
fields: Map<string, Array<FieldDetails>>,
101113
visitedFragmentNames: Set<string>,
102-
localVariableValues?: { [variable: string]: unknown },
114+
fragmentVariables?: FragmentVariables,
103115
): void {
104116
for (const selection of selectionSet.selections) {
105117
switch (selection.kind) {
106118
case Kind.FIELD: {
107-
const vars = localVariableValues ?? variableValues;
108-
if (!shouldIncludeNode(vars, selection)) {
119+
if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
109120
continue;
110121
}
111122
const name = getFieldEntryKey(selection);
112123
const fieldList = fields.get(name);
113124
if (fieldList !== undefined) {
114125
fieldList.push({
115126
node: selection,
116-
fragmentVariableValues: localVariableValues ?? undefined,
127+
fragmentVariables,
117128
});
118129
} else {
119130
fields.set(name, [
120131
{
121132
node: selection,
122-
fragmentVariableValues: localVariableValues ?? undefined,
133+
fragmentVariables,
123134
},
124135
]);
125136
}
126137
break;
127138
}
128139
case Kind.INLINE_FRAGMENT: {
129140
if (
130-
!shouldIncludeNode(variableValues, selection) ||
141+
!shouldIncludeNode(selection, variableValues, fragmentVariables) ||
131142
!doesFragmentConditionMatch(schema, selection, runtimeType)
132143
) {
133144
continue;
@@ -140,54 +151,50 @@ function collectFieldsImpl(
140151
selection.selectionSet,
141152
fields,
142153
visitedFragmentNames,
154+
fragmentVariables,
143155
);
144156
break;
145157
}
146158
case Kind.FRAGMENT_SPREAD: {
147159
const fragName = selection.name.value;
148160
if (
149161
visitedFragmentNames.has(fragName) ||
150-
!shouldIncludeNode(variableValues, selection)
162+
!shouldIncludeNode(selection, variableValues, fragmentVariables)
151163
) {
152164
continue;
153165
}
154166
visitedFragmentNames.add(fragName);
155167
const fragment = fragments[fragName];
156168
if (
157169
!fragment ||
158-
!doesFragmentConditionMatch(schema, fragment, runtimeType)
170+
!doesFragmentConditionMatch(schema, fragment.definition, runtimeType)
159171
) {
160172
continue;
161173
}
162174

163-
// We need to introduce a concept of shadowing:
164-
//
165-
// - when a fragment defines a variable that is in the parent scope but not given
166-
// in the fragment-spread we need to look at this variable as undefined and check
167-
// whether the definition has a defaultValue, if not remove it from the variableValues.
168-
// - when a fragment does not define a variable we need to copy it over from the parent
169-
// scope as that variable can still get used in spreads later on in the selectionSet.
170-
// - when a value is passed in through the fragment-spread we need to copy over the key-value
171-
// into our variable-values.
172-
const fragmentVariableValues = fragment.variableDefinitions
173-
? getArgumentValuesFromSpread(
175+
const fragmentVariableSignatures = fragment.variableSignatures;
176+
let newFragmentVariables: FragmentVariables | undefined;
177+
if (fragmentVariableSignatures) {
178+
newFragmentVariables = {
179+
signatures: fragmentVariableSignatures,
180+
values: experimentalGetArgumentValues(
174181
selection,
175-
schema,
176-
fragment.variableDefinitions,
182+
Object.values(fragmentVariableSignatures),
177183
variableValues,
178-
localVariableValues,
179-
)
180-
: undefined;
184+
fragmentVariables,
185+
),
186+
};
187+
}
181188

182189
collectFieldsImpl(
183190
schema,
184191
fragments,
185192
variableValues,
186193
runtimeType,
187-
fragment.selectionSet,
194+
fragment.definition.selectionSet,
188195
fields,
189196
visitedFragmentNames,
190-
fragmentVariableValues,
197+
newFragmentVariables,
191198
);
192199
break;
193200
}
@@ -200,10 +207,16 @@ function collectFieldsImpl(
200207
* directives, where `@skip` has higher precedence than `@include`.
201208
*/
202209
function shouldIncludeNode(
203-
variableValues: { [variable: string]: unknown },
204210
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
211+
variableValues: { [variable: string]: unknown },
212+
fragmentVariables: FragmentVariables | undefined,
205213
): boolean {
206-
const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);
214+
const skip = getDirectiveValues(
215+
GraphQLSkipDirective,
216+
node,
217+
variableValues,
218+
fragmentVariables,
219+
);
207220
if (skip?.if === true) {
208221
return false;
209222
}
@@ -212,6 +225,7 @@ function shouldIncludeNode(
212225
GraphQLIncludeDirective,
213226
node,
214227
variableValues,
228+
fragmentVariables,
215229
);
216230
if (include?.if === false) {
217231
return false;

‎src/execution/execute.ts

+24-10
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { invariant } from '../jsutils/invariant';
44
import { isIterableObject } from '../jsutils/isIterableObject';
55
import { isObjectLike } from '../jsutils/isObjectLike';
66
import { isPromise } from '../jsutils/isPromise';
7+
import { mapValue } from '../jsutils/mapValue';
78
import type { Maybe } from '../jsutils/Maybe';
89
import { memoize3 } from '../jsutils/memoize3';
910
import type { ObjMap } from '../jsutils/ObjMap';
@@ -20,7 +21,6 @@ import { locatedError } from '../error/locatedError';
2021
import type {
2122
DocumentNode,
2223
FieldNode,
23-
FragmentDefinitionNode,
2424
OperationDefinitionNode,
2525
} from '../language/ast';
2626
import { OperationTypeNode } from '../language/ast';
@@ -52,12 +52,14 @@ import {
5252
import type { GraphQLSchema } from '../type/schema';
5353
import { assertValidSchema } from '../type/validate';
5454

55-
import type { FieldDetails } from './collectFields';
55+
import { getVariableSignature } from '../utilities/getVariableSignature';
56+
57+
import type { FieldDetails, FragmentDetails } from './collectFields';
5658
import {
5759
collectFields,
5860
collectSubfields as _collectSubfields,
5961
} from './collectFields';
60-
import { getArgumentValues, getVariableValues } from './values';
62+
import { experimentalGetArgumentValues, getVariableValues } from './values';
6163

6264
/**
6365
* A memoized collection of relevant subfields with regard to the return
@@ -107,7 +109,7 @@ const collectSubfields = memoize3(
107109
*/
108110
export interface ExecutionContext {
109111
schema: GraphQLSchema;
110-
fragments: ObjMap<FragmentDefinitionNode>;
112+
fragments: ObjMap<FragmentDetails>;
111113
rootValue: unknown;
112114
contextValue: unknown;
113115
operation: OperationDefinitionNode;
@@ -290,7 +292,7 @@ export function buildExecutionContext(
290292
} = args;
291293

292294
let operation: OperationDefinitionNode | undefined;
293-
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
295+
const fragments: ObjMap<FragmentDetails> = Object.create(null);
294296
for (const definition of document.definitions) {
295297
switch (definition.kind) {
296298
case Kind.OPERATION_DEFINITION:
@@ -307,9 +309,18 @@ export function buildExecutionContext(
307309
operation = definition;
308310
}
309311
break;
310-
case Kind.FRAGMENT_DEFINITION:
311-
fragments[definition.name.value] = definition;
312+
case Kind.FRAGMENT_DEFINITION: {
313+
let variableSignatures;
314+
if (definition.variableDefinitions) {
315+
variableSignatures = Object.create(null);
316+
for (const varDef of definition.variableDefinitions) {
317+
const signature = getVariableSignature(schema, varDef);
318+
variableSignatures[signature.name] = signature;
319+
}
320+
}
321+
fragments[definition.name.value] = { definition, variableSignatures };
312322
break;
323+
}
313324
default:
314325
// ignore non-executable definitions
315326
}
@@ -519,11 +530,11 @@ function executeField(
519530
// Build a JS object of arguments from the field.arguments AST, using the
520531
// variables scope to fulfill any variable references.
521532
// TODO: find a way to memoize, in case this field is within a List type.
522-
const args = getArgumentValues(
533+
const args = experimentalGetArgumentValues(
523534
fieldEntries[0].node,
524535
fieldDef.args,
525536
exeContext.variableValues,
526-
fieldEntries[0].fragmentVariableValues,
537+
fieldEntries[0].fragmentVariables,
527538
);
528539

529540
// The resolve function's optional third argument is a context value that
@@ -598,7 +609,10 @@ export function buildResolveInfo(
598609
parentType,
599610
path,
600611
schema: exeContext.schema,
601-
fragments: exeContext.fragments,
612+
fragments: mapValue(
613+
exeContext.fragments,
614+
(fragment) => fragment.definition,
615+
),
602616
rootValue: exeContext.rootValue,
603617
operation: exeContext.operation,
604618
variableValues: exeContext.variableValues,

0 commit comments

Comments
 (0)