Skip to content

Commit 8b2a56d

Browse files
mjmahonemjmahone
committed
Fragment Args rebased for 2022
1 parent 40bf7ce commit 8b2a56d

File tree

11 files changed

+313
-20
lines changed

11 files changed

+313
-20
lines changed

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

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,166 @@ describe('Execute: Handles inputs', () => {
10061006
});
10071007
});
10081008

1009+
describe('using fragment arguments', () => {
1010+
it('when there are no fragment arguments', () => {
1011+
const result = executeQuery(`
1012+
query {
1013+
...a
1014+
}
1015+
1016+
fragment a on TestType {
1017+
fieldWithNonNullableStringInput(input: "A")
1018+
}
1019+
`);
1020+
expect(result).to.deep.equal({
1021+
data: {
1022+
fieldWithNonNullableStringInput: '"A"',
1023+
},
1024+
});
1025+
});
1026+
1027+
it('when a value is required and provided', () => {
1028+
const result = executeQuery(`
1029+
query {
1030+
...a(value: "A")
1031+
}
1032+
1033+
fragment a($value: String!) on TestType {
1034+
fieldWithNonNullableStringInput(input: $value)
1035+
}
1036+
`);
1037+
expect(result).to.deep.equal({
1038+
data: {
1039+
fieldWithNonNullableStringInput: '"A"',
1040+
},
1041+
});
1042+
});
1043+
1044+
it('when a value is required and not provided', () => {
1045+
const result = executeQuery(`
1046+
query {
1047+
...a
1048+
}
1049+
1050+
fragment a($value: String!) on TestType {
1051+
fieldWithNullableStringInput(input: $value)
1052+
}
1053+
`);
1054+
expect(result).to.deep.equal({
1055+
data: {
1056+
fieldWithNullableStringInput: null,
1057+
},
1058+
});
1059+
});
1060+
1061+
it('when the definition has a default and is provided', () => {
1062+
const result = executeQuery(`
1063+
query {
1064+
...a(value: "A")
1065+
}
1066+
1067+
fragment a($value: String! = "B") on TestType {
1068+
fieldWithNonNullableStringInput(input: $value)
1069+
}
1070+
`);
1071+
expect(result).to.deep.equal({
1072+
data: {
1073+
fieldWithNonNullableStringInput: '"A"',
1074+
},
1075+
});
1076+
});
1077+
1078+
it('when the definition has a default and is not provided', () => {
1079+
const result = executeQuery(`
1080+
query {
1081+
...a
1082+
}
1083+
1084+
fragment a($value: String! = "B") on TestType {
1085+
fieldWithNonNullableStringInput(input: $value)
1086+
}
1087+
`);
1088+
expect(result).to.deep.equal({
1089+
data: {
1090+
fieldWithNonNullableStringInput: '"B"',
1091+
},
1092+
});
1093+
});
1094+
1095+
it('when the definition has a non-nullable default and is provided null', () => {
1096+
const result = executeQuery(`
1097+
query {
1098+
...a(value: null)
1099+
}
1100+
1101+
fragment a($value: String! = "B") on TestType {
1102+
fieldWithNullableStringInput(input: $value)
1103+
}
1104+
`);
1105+
expect(result).to.deep.equal({
1106+
data: {
1107+
fieldWithNullableStringInput: 'null',
1108+
},
1109+
});
1110+
});
1111+
1112+
it('when the definition has no default and is not provided', () => {
1113+
const result = executeQuery(`
1114+
query {
1115+
...a
1116+
}
1117+
1118+
fragment a($value: String) on TestType {
1119+
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
1120+
}
1121+
`);
1122+
expect(result).to.deep.equal({
1123+
data: {
1124+
fieldWithNonNullableStringInputAndDefaultArgumentValue:
1125+
'"Hello World"',
1126+
},
1127+
});
1128+
});
1129+
1130+
it('when the argument variable is nested in a complex type', () => {
1131+
const result = executeQuery(`
1132+
query {
1133+
...a(value: "C")
1134+
}
1135+
1136+
fragment a($value: String) on TestType {
1137+
list(input: ["A", "B", $value, "D"])
1138+
}
1139+
`);
1140+
expect(result).to.deep.equal({
1141+
data: {
1142+
list: '["A", "B", "C", "D"]',
1143+
},
1144+
});
1145+
});
1146+
1147+
it('when argument variables are used recursively', () => {
1148+
const result = executeQuery(`
1149+
query {
1150+
...a(aValue: "C")
1151+
}
1152+
1153+
fragment a($aValue: String) on TestType {
1154+
...b(bValue: $aValue)
1155+
}
1156+
1157+
fragment b($bValue: String) on TestType {
1158+
list(input: ["A", "B", $bValue, "D"])
1159+
}
1160+
`);
1161+
expect(result).to.deep.equal({
1162+
data: {
1163+
list: '["A", "B", "C", "D"]',
1164+
},
1165+
});
1166+
});
1167+
});
1168+
10091169
describe('getVariableValues: limit maximum number of coercion errors', () => {
10101170
const doc = parse(`
10111171
query ($input: [String!]) {

‎src/execution/collectFields.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import type { GraphQLSchema } from '../type/schema';
2020

2121
import { typeFromAST } from '../utilities/typeFromAST';
2222

23+
import { visit } from '../language/visitor';
24+
2325
import { getDirectiveValues } from './values';
2426

2527
/**
@@ -144,7 +146,7 @@ function collectFieldsImpl(
144146
fragments,
145147
variableValues,
146148
runtimeType,
147-
fragment.selectionSet,
149+
selectionSetWithArgumentsApplied,
148150
fields,
149151
visitedFragmentNames,
150152
);
@@ -206,3 +208,44 @@ function doesFragmentConditionMatch(
206208
function getFieldEntryKey(node: FieldNode): string {
207209
return node.alias ? node.alias.value : node.name.value;
208210
}
211+
212+
/**
213+
*
214+
* When a fragment spread is provided with arguments,
215+
* visit that fragment's definition and replace those arguments'
216+
* variable usages with the provided argument value.
217+
*/
218+
function selectionSetWithFragmentArgumentsApplied(
219+
fragment: FragmentDefinitionNode,
220+
fragmentSpread: FragmentSpreadNode,
221+
): SelectionSetNode {
222+
const variableDefinitions = fragment.variableDefinitions;
223+
if (!variableDefinitions) {
224+
return fragment.selectionSet;
225+
}
226+
227+
const providedArguments: Map<string, ArgumentNode> = new Map();
228+
if (fragmentSpread.arguments) {
229+
for (const argument of fragmentSpread.arguments) {
230+
providedArguments.set(argument.name.value, argument);
231+
}
232+
}
233+
234+
const fragmentVariableValues: Map<string, ValueNode> = new Map();
235+
for (const variableDef of variableDefinitions) {
236+
const variableName = variableDef.variable.name.value;
237+
const providedArg = providedArguments.get(variableName);
238+
if (providedArg) {
239+
fragmentVariableValues.set(variableName, providedArg.value);
240+
} else if (variableDef.defaultValue) {
241+
fragmentVariableValues.set(variableName, variableDef.defaultValue);
242+
}
243+
// Otherwise just preserve the variable as-is: it will be treated as unset by the executor.
244+
}
245+
246+
return visit(fragment.selectionSet, {
247+
Variable(node) {
248+
return fragmentVariableValues.get(node.name.value);
249+
},
250+
});
251+
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -594,19 +594,13 @@ describe('Parser', () => {
594594
it('allows parsing fragment defined variables', () => {
595595
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
596596

597-
expect(() =>
598-
parse(document, { allowFragmentArguments: true }),
599-
).to.not.throw();
600-
expect(() => parse(document)).to.throw('Syntax Error');
597+
expect(() => parse(document)).to.not.throw();
601598
});
602599

603600
it('allows parsing fragment spread arguments', () => {
604601
const document = 'fragment a on t { ...b(v: $v) }';
605602

606-
expect(() =>
607-
parse(document, { allowFragmentArguments: true }),
608-
).to.not.throw();
609-
expect(() => parse(document)).to.throw('Syntax Error');
603+
expect(() => parse(document)).to.not.throw();
610604
});
611605

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

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ describe('Printer: Query document', () => {
113113
it('prints fragment with variable directives', () => {
114114
const queryASTWithVariableDirective = parse(
115115
'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
116-
{ allowFragmentArguments: true },
117116
);
118117
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
119118
fragment Foo($foo: TestType @test) on TestType @testDirective {
@@ -129,7 +128,6 @@ describe('Printer: Query document', () => {
129128
id
130129
}
131130
`,
132-
{ allowFragmentArguments: true },
133131
);
134132
expect(print(fragmentWithVariable)).to.equal(dedent`
135133
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
@@ -141,7 +139,6 @@ describe('Printer: Query document', () => {
141139
it('prints fragment spread with arguments', () => {
142140
const queryASTWithVariableDirective = parse(
143141
'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
144-
{ allowFragmentArguments: true },
145142
);
146143
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
147144
fragment Foo on TestType {
@@ -153,7 +150,6 @@ describe('Printer: Query document', () => {
153150
it('prints fragment spread with multi-line arguments', () => {
154151
const queryASTWithVariableDirective = parse(
155152
'fragment Foo on TestType { ...Bar(a: {x: $x, y: $y, z: $z, xy: $xy}, b: true, c: "a long string extending arguments over max length") }',
156-
{ allowFragmentArguments: true },
157153
);
158154
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
159155
fragment Foo on TestType {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,6 @@ describe('Visitor', () => {
458458
it('visits variables defined in fragments', () => {
459459
const ast = parse('fragment a($v: Boolean = false) on t { f }', {
460460
noLocation: true,
461-
allowFragmentArguments: true,
462461
});
463462
const visited: Array<any> = [];
464463

‎src/language/parser.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ export interface ParseOptions {
8080
* in the source that they correspond to. This configuration flag
8181
* disables that behavior for performance or testing.
8282
*/
83+
<<<<<<< HEAD
84+
<<<<<<< HEAD
8385
noLocation?: boolean | undefined;
8486

8587
/**
@@ -123,6 +125,12 @@ export interface ParseOptions {
123125
* future.
124126
*/
125127
experimentalClientControlledNullability?: boolean | undefined;
128+
=======
129+
noLocation?: boolean;
130+
>>>>>>> Remove flag for parsing, but add default validation to indicate fragment args are not yet spec-supported, and may be missing validation.
131+
=======
132+
noLocation?: boolean;
133+
>>>>>>> 01414be319c245415c1c5d4818a4fe9179c2909d
126134
}
127135

128136
/**
@@ -551,17 +559,18 @@ export class Parser {
551559

552560
const hasTypeCondition = this.expectOptionalKeyword('on');
553561
if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
554-
if (this._options?.allowFragmentArguments === true) {
562+
const name = this.parseFragmentName();
563+
if (this.peek(TokenKind.PAREN_L)) {
555564
return this.node<FragmentSpreadNode>(start, {
556565
kind: Kind.FRAGMENT_SPREAD,
557-
name: this.parseFragmentName(),
566+
name,
558567
arguments: this.parseArguments(false),
559568
directives: this.parseDirectives(false),
560569
});
561570
}
562571
return this.node<FragmentSpreadNode>(start, {
563572
kind: Kind.FRAGMENT_SPREAD,
564-
name: this.parseFragmentName(),
573+
name,
565574
directives: this.parseDirectives(false),
566575
});
567576
}
@@ -588,7 +597,7 @@ export class Parser {
588597
if (this._options.allowLegacyFragmentVariables === true) {
589598
return this.node<FragmentDefinitionNode>(start, {
590599
kind: Kind.FRAGMENT_DEFINITION,
591-
name: this.parseFragmentName(),
600+
name,
592601
variableDefinitions: this.parseVariableDefinitions(),
593602
typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
594603
directives: this.parseDirectives(false),
@@ -597,7 +606,7 @@ export class Parser {
597606
}
598607
return this.node<FragmentDefinitionNode>(start, {
599608
kind: Kind.FRAGMENT_DEFINITION,
600-
name: this.parseFragmentName(),
609+
name,
601610
typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
602611
directives: this.parseDirectives(false),
603612
selectionSet: this.parseSelectionSet(),

‎src/utilities/buildASTSchema.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ export function buildSchema(
9393
): GraphQLSchema {
9494
const document = parse(source, {
9595
noLocation: options?.noLocation,
96-
allowFragmentArguments: options?.allowFragmentArguments,
9796
});
9897

9998
return buildASTSchema(document, {

0 commit comments

Comments
 (0)