Skip to content

Commit 922839c

Browse files
mjmahonemjmahone
committed
RFC: Fragment Arguments
1 parent 8be83d8 commit 922839c

32 files changed

+1665
-266
lines changed

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

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ function fieldWithInputArg(
8282
};
8383
}
8484

85+
const NestedType: GraphQLObjectType = new GraphQLObjectType({
86+
name: 'NestedType',
87+
fields: {
88+
echo: fieldWithInputArg({ type: GraphQLString }),
89+
},
90+
});
91+
8592
const TestType = new GraphQLObjectType({
8693
name: 'TestType',
8794
fields: {
@@ -107,6 +114,10 @@ const TestType = new GraphQLObjectType({
107114
defaultValue: 'Hello World',
108115
}),
109116
list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
117+
nested: {
118+
type: NestedType,
119+
resolve: () => ({}),
120+
},
110121
nnList: fieldWithInputArg({
111122
type: new GraphQLNonNull(new GraphQLList(GraphQLString)),
112123
}),
@@ -1006,6 +1017,245 @@ describe('Execute: Handles inputs', () => {
10061017
});
10071018
});
10081019

1020+
describe('using fragment arguments', () => {
1021+
it('when there are no fragment arguments', () => {
1022+
const result = executeQuery(`
1023+
query {
1024+
...a
1025+
}
1026+
fragment a on TestType {
1027+
fieldWithNonNullableStringInput(input: "A")
1028+
}
1029+
`);
1030+
expect(result).to.deep.equal({
1031+
data: {
1032+
fieldWithNonNullableStringInput: '"A"',
1033+
},
1034+
});
1035+
});
1036+
1037+
it('when a value is required and provided', () => {
1038+
const result = executeQuery(`
1039+
query {
1040+
...a(value: "A")
1041+
}
1042+
fragment a($value: String!) on TestType {
1043+
fieldWithNonNullableStringInput(input: $value)
1044+
}
1045+
`);
1046+
expect(result).to.deep.equal({
1047+
data: {
1048+
fieldWithNonNullableStringInput: '"A"',
1049+
},
1050+
});
1051+
});
1052+
1053+
it('when a value is required and not provided', () => {
1054+
const result = executeQuery(`
1055+
query {
1056+
...a
1057+
}
1058+
fragment a($value: String!) on TestType {
1059+
fieldWithNullableStringInput(input: $value)
1060+
}
1061+
`);
1062+
expect(result).to.deep.equal({
1063+
data: {
1064+
fieldWithNullableStringInput: null,
1065+
},
1066+
});
1067+
});
1068+
1069+
it('when the definition has a default and is provided', () => {
1070+
const result = executeQuery(`
1071+
query {
1072+
...a(value: "A")
1073+
}
1074+
fragment a($value: String! = "B") on TestType {
1075+
fieldWithNonNullableStringInput(input: $value)
1076+
}
1077+
`);
1078+
expect(result).to.deep.equal({
1079+
data: {
1080+
fieldWithNonNullableStringInput: '"A"',
1081+
},
1082+
});
1083+
});
1084+
1085+
it('when the definition has a default and is not provided', () => {
1086+
const result = executeQuery(`
1087+
query {
1088+
...a
1089+
}
1090+
fragment a($value: String! = "B") on TestType {
1091+
fieldWithNonNullableStringInput(input: $value)
1092+
}
1093+
`);
1094+
expect(result).to.deep.equal({
1095+
data: {
1096+
fieldWithNonNullableStringInput: '"B"',
1097+
},
1098+
});
1099+
});
1100+
1101+
it('when the definition has a non-nullable default and is provided null', () => {
1102+
const result = executeQuery(`
1103+
query {
1104+
...a(value: null)
1105+
}
1106+
fragment a($value: String! = "B") on TestType {
1107+
fieldWithNullableStringInput(input: $value)
1108+
}
1109+
`);
1110+
expect(result).to.deep.equal({
1111+
data: {
1112+
fieldWithNullableStringInput: 'null',
1113+
},
1114+
});
1115+
});
1116+
1117+
it('when the definition has no default and is not provided', () => {
1118+
const result = executeQuery(`
1119+
query {
1120+
...a
1121+
}
1122+
fragment a($value: String) on TestType {
1123+
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
1124+
}
1125+
`);
1126+
expect(result).to.deep.equal({
1127+
data: {
1128+
fieldWithNonNullableStringInputAndDefaultArgumentValue:
1129+
'"Hello World"',
1130+
},
1131+
});
1132+
});
1133+
1134+
it('when an argument is shadowed by an operation variable', () => {
1135+
const result = executeQuery(`
1136+
query($x: String! = "A") {
1137+
...a(x: "B")
1138+
}
1139+
fragment a($x: String) on TestType {
1140+
fieldWithNullableStringInput(input: $x)
1141+
}
1142+
`);
1143+
expect(result).to.deep.equal({
1144+
data: {
1145+
fieldWithNullableStringInput: '"B"',
1146+
},
1147+
});
1148+
});
1149+
1150+
it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
1151+
const result = executeQuery(`
1152+
query($x: String = "A") {
1153+
...a
1154+
}
1155+
fragment a($x: String) on TestType {
1156+
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $x)
1157+
}
1158+
`);
1159+
expect(result).to.deep.equal({
1160+
data: {
1161+
fieldWithNonNullableStringInputAndDefaultArgumentValue:
1162+
'"Hello World"',
1163+
},
1164+
});
1165+
});
1166+
1167+
it('when a fragment is used with different args', () => {
1168+
const result = executeQuery(`
1169+
query($x: String = "Hello") {
1170+
a: nested {
1171+
...a(x: "a")
1172+
}
1173+
b: nested {
1174+
...a(x: "b", b: true)
1175+
}
1176+
hello: nested {
1177+
...a(x: $x)
1178+
}
1179+
}
1180+
fragment a($x: String, $b: Boolean = false) on NestedType {
1181+
a: echo(input: $x) @skip(if: $b)
1182+
b: echo(input: $x) @include(if: $b)
1183+
}
1184+
`);
1185+
expect(result).to.deep.equal({
1186+
data: {
1187+
a: {
1188+
a: '"a"',
1189+
},
1190+
b: {
1191+
b: '"b"',
1192+
},
1193+
hello: {
1194+
a: '"Hello"',
1195+
},
1196+
},
1197+
});
1198+
});
1199+
1200+
it('when the argument variable is nested in a complex type', () => {
1201+
const result = executeQuery(`
1202+
query {
1203+
...a(value: "C")
1204+
}
1205+
fragment a($value: String) on TestType {
1206+
list(input: ["A", "B", $value, "D"])
1207+
}
1208+
`);
1209+
expect(result).to.deep.equal({
1210+
data: {
1211+
list: '["A", "B", "C", "D"]',
1212+
},
1213+
});
1214+
});
1215+
1216+
it('when argument variables are used recursively', () => {
1217+
const result = executeQuery(`
1218+
query {
1219+
...a(aValue: "C")
1220+
}
1221+
fragment a($aValue: String) on TestType {
1222+
...b(bValue: $aValue)
1223+
}
1224+
fragment b($bValue: String) on TestType {
1225+
list(input: ["A", "B", $bValue, "D"])
1226+
}
1227+
`);
1228+
expect(result).to.deep.equal({
1229+
data: {
1230+
list: '["A", "B", "C", "D"]',
1231+
},
1232+
});
1233+
});
1234+
1235+
it('when argument passed in as list', () => {
1236+
const result = executeQuery(`
1237+
query Q($opValue: String = "op") {
1238+
...a(aValue: "A")
1239+
}
1240+
fragment a($aValue: String, $bValue: String) on TestType {
1241+
...b(aValue: [$aValue, "B"], bValue: [$bValue, $opValue])
1242+
}
1243+
fragment b($aValue: [String], $bValue: [String], $cValue: String) on TestType {
1244+
aList: list(input: $aValue)
1245+
bList: list(input: $bValue)
1246+
cList: list(input: [$cValue])
1247+
}
1248+
`);
1249+
expect(result).to.deep.equal({
1250+
data: {
1251+
aList: '["A", "B"]',
1252+
bList: '[null, "op"]',
1253+
cList: '[null]',
1254+
},
1255+
});
1256+
});
1257+
});
1258+
10091259
describe('getVariableValues: limit maximum number of coercion errors', () => {
10101260
const doc = parse(`
10111261
query ($input: [String!]) {

‎src/execution/collectFields.ts

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

25+
import { keyForFragmentSpread } from '../utilities/keyForFragmentSpread.js';
26+
import { substituteFragmentArguments } from '../utilities/substituteFragmentArguments.js';
2527
import { typeFromAST } from '../utilities/typeFromAST.js';
2628

2729
import { getDirectiveValues } from './values.js';
@@ -124,7 +126,7 @@ function collectFieldsImpl(
124126
selectionSet: SelectionSetNode,
125127
fields: AccumulatorMap<string, FieldNode>,
126128
patches: Array<PatchFields>,
127-
visitedFragmentNames: Set<string>,
129+
visitedFragmentKeys: Set<string>,
128130
): void {
129131
for (const selection of selectionSet.selections) {
130132
switch (selection.kind) {
@@ -156,7 +158,7 @@ function collectFieldsImpl(
156158
selection.selectionSet,
157159
patchFields,
158160
patches,
159-
visitedFragmentNames,
161+
visitedFragmentKeys,
160162
);
161163
patches.push({
162164
label: defer.label,
@@ -172,24 +174,24 @@ function collectFieldsImpl(
172174
selection.selectionSet,
173175
fields,
174176
patches,
175-
visitedFragmentNames,
177+
visitedFragmentKeys,
176178
);
177179
}
178180
break;
179181
}
180182
case Kind.FRAGMENT_SPREAD: {
181-
const fragName = selection.name.value;
183+
const fragmentKey = keyForFragmentSpread(selection);
182184

183185
if (!shouldIncludeNode(variableValues, selection)) {
184186
continue;
185187
}
186188

187189
const defer = getDeferValues(operation, variableValues, selection);
188-
if (visitedFragmentNames.has(fragName) && !defer) {
190+
if (visitedFragmentKeys.has(fragmentKey) && !defer) {
189191
continue;
190192
}
191193

192-
const fragment = fragments[fragName];
194+
const fragment = fragments[selection.name.value];
193195
if (
194196
!fragment ||
195197
!doesFragmentConditionMatch(schema, fragment, runtimeType)
@@ -198,9 +200,13 @@ function collectFieldsImpl(
198200
}
199201

200202
if (!defer) {
201-
visitedFragmentNames.add(fragName);
203+
visitedFragmentKeys.add(fragmentKey);
202204
}
203205

206+
const fragmentSelectionSet = substituteFragmentArguments(
207+
fragment,
208+
selection,
209+
);
204210
if (defer) {
205211
const patchFields = new AccumulatorMap<string, FieldNode>();
206212
collectFieldsImpl(
@@ -209,10 +215,10 @@ function collectFieldsImpl(
209215
variableValues,
210216
operation,
211217
runtimeType,
212-
fragment.selectionSet,
218+
fragmentSelectionSet,
213219
patchFields,
214220
patches,
215-
visitedFragmentNames,
221+
visitedFragmentKeys,
216222
);
217223
patches.push({
218224
label: defer.label,
@@ -225,10 +231,10 @@ function collectFieldsImpl(
225231
variableValues,
226232
operation,
227233
runtimeType,
228-
fragment.selectionSet,
234+
fragmentSelectionSet,
229235
fields,
230236
patches,
231-
visitedFragmentNames,
237+
visitedFragmentKeys,
232238
);
233239
}
234240
break;

‎src/language/__tests__/parser-test.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -607,13 +607,16 @@ describe('Parser', () => {
607607
expect('loc' in result).to.equal(false);
608608
});
609609

610-
it('Legacy: allows parsing fragment defined variables', () => {
610+
it('allows parsing fragment defined arguments', () => {
611611
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
612612

613-
expect(() =>
614-
parse(document, { allowLegacyFragmentVariables: true }),
615-
).to.not.throw();
616-
expect(() => parse(document)).to.throw('Syntax Error');
613+
expect(() => parse(document)).to.not.throw();
614+
});
615+
616+
it('allows parsing fragment spread arguments', () => {
617+
const document = 'fragment a on t { ...b(v: $v) }';
618+
619+
expect(() => parse(document)).to.not.throw();
617620
});
618621

619622
it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {

0 commit comments

Comments
 (0)