Skip to content

Commit 839284d

Browse files
JoviDeCroockmjmahone
JoviDeCroock
and
mjmahone
committed
Add support for parsing fragment arguments
Co-authored-by: mjmahone <mahoney.mattj@gmail.com>
1 parent 5c7d4d1 commit 839284d

File tree

14 files changed

+188
-47
lines changed

14 files changed

+188
-47
lines changed

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -607,13 +607,32 @@ 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 variables', () => {
611611
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
612612

613613
expect(() =>
614-
parse(document, { allowLegacyFragmentVariables: true }),
614+
parse(document, { experimentalFragmentArguments: true }),
615615
).to.not.throw();
616-
expect(() => parse(document)).to.throw('Syntax Error');
616+
});
617+
618+
it('disallows parsing fragment defined variables without experimental flag', () => {
619+
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
620+
621+
expect(() => parse(document)).to.throw();
622+
});
623+
624+
it('allows parsing fragment spread arguments', () => {
625+
const document = 'fragment a on t { ...b(v: $v) }';
626+
627+
expect(() =>
628+
parse(document, { experimentalFragmentArguments: true }),
629+
).to.not.throw();
630+
});
631+
632+
it('disallows parsing fragment spread arguments without experimental flag', () => {
633+
const document = 'fragment a on t { ...b(v: $v) }';
634+
635+
expect(() => parse(document)).to.throw();
617636
});
618637

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

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

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,34 +168,62 @@ describe('Printer: Query document', () => {
168168
`);
169169
});
170170

171-
it('Legacy: prints fragment with variable directives', () => {
172-
const queryASTWithVariableDirective = parse(
171+
it('prints fragment with argument definition directives', () => {
172+
const fragmentWithArgumentDefinitionDirective = parse(
173173
'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
174-
{ allowLegacyFragmentVariables: true },
174+
{ experimentalFragmentArguments: true },
175175
);
176-
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
176+
expect(print(fragmentWithArgumentDefinitionDirective)).to.equal(dedent`
177177
fragment Foo($foo: TestType @test) on TestType @testDirective {
178178
id
179179
}
180180
`);
181181
});
182182

183-
it('Legacy: correctly prints fragment defined variables', () => {
184-
const fragmentWithVariable = parse(
183+
it('correctly prints fragment defined arguments', () => {
184+
const fragmentWithArgumentDefinition = parse(
185185
`
186186
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
187187
id
188188
}
189189
`,
190-
{ allowLegacyFragmentVariables: true },
190+
{ experimentalFragmentArguments: true },
191191
);
192-
expect(print(fragmentWithVariable)).to.equal(dedent`
192+
expect(print(fragmentWithArgumentDefinition)).to.equal(dedent`
193193
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
194194
id
195195
}
196196
`);
197197
});
198198

199+
it('prints fragment spread with arguments', () => {
200+
const fragmentSpreadWithArguments = parse(
201+
'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
202+
{ experimentalFragmentArguments: true },
203+
);
204+
expect(print(fragmentSpreadWithArguments)).to.equal(dedent`
205+
fragment Foo on TestType {
206+
...Bar(a: { x: $x }, b: true)
207+
}
208+
`);
209+
});
210+
211+
it('prints fragment spread with multi-line arguments', () => {
212+
const fragmentSpreadWithArguments = parse(
213+
'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") }',
214+
{ experimentalFragmentArguments: true },
215+
);
216+
expect(print(fragmentSpreadWithArguments)).to.equal(dedent`
217+
fragment Foo on TestType {
218+
...Bar(
219+
a: { x: $x, y: $y, z: $z, xy: $xy }
220+
b: true
221+
c: "a long string extending arguments over max length"
222+
)
223+
}
224+
`);
225+
});
226+
199227
it('prints kitchen sink without altering ast', () => {
200228
const ast = parse(kitchenSinkQuery, {
201229
noLocation: true,

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

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,10 +455,10 @@ describe('Visitor', () => {
455455
]);
456456
});
457457

458-
it('Legacy: visits variables defined in fragments', () => {
458+
it('visits arguments defined on fragments', () => {
459459
const ast = parse('fragment a($v: Boolean = false) on t { f }', {
460460
noLocation: true,
461-
allowLegacyFragmentVariables: true,
461+
experimentalFragmentArguments: true,
462462
});
463463
const visited: Array<any> = [];
464464

@@ -505,6 +505,50 @@ describe('Visitor', () => {
505505
]);
506506
});
507507

508+
it('visits arguments on fragment spreads', () => {
509+
const ast = parse('fragment a on t { ...s(v: false) }', {
510+
noLocation: true,
511+
experimentalFragmentArguments: true,
512+
});
513+
const visited: Array<any> = [];
514+
515+
visit(ast, {
516+
enter(node) {
517+
checkVisitorFnArgs(ast, arguments);
518+
visited.push(['enter', node.kind, getValue(node)]);
519+
},
520+
leave(node) {
521+
checkVisitorFnArgs(ast, arguments);
522+
visited.push(['leave', node.kind, getValue(node)]);
523+
},
524+
});
525+
526+
expect(visited).to.deep.equal([
527+
['enter', 'Document', undefined],
528+
['enter', 'FragmentDefinition', undefined],
529+
['enter', 'Name', 'a'],
530+
['leave', 'Name', 'a'],
531+
['enter', 'NamedType', undefined],
532+
['enter', 'Name', 't'],
533+
['leave', 'Name', 't'],
534+
['leave', 'NamedType', undefined],
535+
['enter', 'SelectionSet', undefined],
536+
['enter', 'FragmentSpread', undefined],
537+
['enter', 'Name', 's'],
538+
['leave', 'Name', 's'],
539+
['enter', 'Argument', { kind: 'BooleanValue', value: false }],
540+
['enter', 'Name', 'v'],
541+
['leave', 'Name', 'v'],
542+
['enter', 'BooleanValue', false],
543+
['leave', 'BooleanValue', false],
544+
['leave', 'Argument', { kind: 'BooleanValue', value: false }],
545+
['leave', 'FragmentSpread', undefined],
546+
['leave', 'SelectionSet', undefined],
547+
['leave', 'FragmentDefinition', undefined],
548+
['leave', 'Document', undefined],
549+
]);
550+
});
551+
508552
it('properly visits the kitchen sink query', () => {
509553
const ast = parse(kitchenSinkQuery, {
510554
experimentalClientControlledNullability: true,

‎src/language/ast.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ export const QueryDocumentKeys: {
227227
NonNullAssertion: ['nullabilityAssertion'],
228228
ErrorBoundary: ['nullabilityAssertion'],
229229

230-
FragmentSpread: ['name', 'directives'],
230+
FragmentSpread: ['name', 'arguments', 'directives'],
231231
InlineFragment: ['typeCondition', 'directives', 'selectionSet'],
232232
FragmentDefinition: [
233233
'name',
@@ -428,6 +428,7 @@ export interface FragmentSpreadNode {
428428
readonly kind: Kind.FRAGMENT_SPREAD;
429429
readonly loc?: Location | undefined;
430430
readonly name: NameNode;
431+
readonly arguments?: ReadonlyArray<ArgumentNode> | undefined;
431432
readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
432433
}
433434

@@ -443,7 +444,6 @@ export interface FragmentDefinitionNode {
443444
readonly kind: Kind.FRAGMENT_DEFINITION;
444445
readonly loc?: Location | undefined;
445446
readonly name: NameNode;
446-
/** @deprecated variableDefinitions will be removed in v17.0.0 */
447447
readonly variableDefinitions?:
448448
| ReadonlyArray<VariableDefinitionNode>
449449
| undefined;

‎src/language/directiveLocation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ export enum DirectiveLocation {
2323
ENUM_VALUE = 'ENUM_VALUE',
2424
INPUT_OBJECT = 'INPUT_OBJECT',
2525
INPUT_FIELD_DEFINITION = 'INPUT_FIELD_DEFINITION',
26+
FRAGMENT_VARIABLE_DEFINITION = 'FRAGMENT_VARIABLE_DEFINITION',
2627
}

‎src/language/parser.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,25 @@ export interface ParseOptions {
9292
maxTokens?: number | undefined;
9393

9494
/**
95-
* @deprecated will be removed in the v17.0.0
95+
* EXPERIMENTAL:
9696
*
97-
* If enabled, the parser will understand and parse variable definitions
98-
* contained in a fragment definition. They'll be represented in the
99-
* `variableDefinitions` field of the FragmentDefinitionNode.
97+
* If enabled, the parser will understand and parse fragment variable definitions
98+
* and arguments on fragment spreads. Fragment variable definitions will be represented
99+
* in the `variableDefinitions` field of the FragmentDefinitionNode.
100+
* Fragment spread arguments will be represented in the `arguments` field of FragmentSpreadNode.
100101
*
101-
* The syntax is identical to normal, query-defined variables. For example:
102+
* For example:
102103
*
103104
* ```graphql
105+
* {
106+
* t { ...A(var: true) }
107+
* }
104108
* fragment A($var: Boolean = false) on T {
105-
* ...
109+
* ...B(x: $var)
106110
* }
107111
* ```
108112
*/
109-
allowLegacyFragmentVariables?: boolean | undefined;
113+
experimentalFragmentArguments?: boolean | undefined;
110114

111115
/**
112116
* EXPERIMENTAL:
@@ -550,7 +554,7 @@ export class Parser {
550554
/**
551555
* Corresponds to both FragmentSpread and InlineFragment in the spec.
552556
*
553-
* FragmentSpread : ... FragmentName Directives?
557+
* FragmentSpread : ... FragmentName Arguments? Directives?
554558
*
555559
* InlineFragment : ... TypeCondition? Directives? SelectionSet
556560
*/
@@ -560,9 +564,21 @@ export class Parser {
560564

561565
const hasTypeCondition = this.expectOptionalKeyword('on');
562566
if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
567+
const name = this.parseFragmentName();
568+
if (
569+
this.peek(TokenKind.PAREN_L) &&
570+
this._options.experimentalFragmentArguments
571+
) {
572+
return this.node<FragmentSpreadNode>(start, {
573+
kind: Kind.FRAGMENT_SPREAD,
574+
name,
575+
arguments: this.parseArguments(false),
576+
directives: this.parseDirectives(false),
577+
});
578+
}
563579
return this.node<FragmentSpreadNode>(start, {
564580
kind: Kind.FRAGMENT_SPREAD,
565-
name: this.parseFragmentName(),
581+
name,
566582
directives: this.parseDirectives(false),
567583
});
568584
}
@@ -576,21 +592,18 @@ export class Parser {
576592

577593
/**
578594
* FragmentDefinition :
579-
* - fragment FragmentName on TypeCondition Directives? SelectionSet
595+
* - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
580596
*
581597
* TypeCondition : NamedType
582598
*/
583599
parseFragmentDefinition(): FragmentDefinitionNode {
584600
const start = this._lexer.token;
585601
this.expectKeyword('fragment');
586-
// Legacy support for defining variables within fragments changes
587-
// the grammar of FragmentDefinition:
588-
// - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
589-
if (this._options.allowLegacyFragmentVariables === true) {
602+
if (this._options.experimentalFragmentArguments === true) {
590603
return this.node<FragmentDefinitionNode>(start, {
591604
kind: Kind.FRAGMENT_DEFINITION,
592605
name: this.parseFragmentName(),
593-
variableDefinitions: this.parseVariableDefinitions(),
606+
variableDefinitions:this.parseVariableDefinitions(),
594607
typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
595608
directives: this.parseDirectives(false),
596609
selectionSet: this.parseSelectionSet(),

‎src/language/printer.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,9 @@ const printDocASTReducer: ASTReducer<string> = {
6464
selectionSet,
6565
}) {
6666
const prefix = join([wrap('', alias, ': '), name], '');
67-
let argsLine = prefix + wrap('(', join(args, ', '), ')');
68-
69-
if (argsLine.length > MAX_LINE_LENGTH) {
70-
argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
71-
}
7267

7368
return join([
74-
argsLine,
69+
wrappedLineAndArgs(prefix, args),
7570
// Note: Client Controlled Nullability is experimental and may be
7671
// changed or removed in the future.
7772
nullabilityAssertion,
@@ -105,8 +100,12 @@ const printDocASTReducer: ASTReducer<string> = {
105100
// Fragments
106101

107102
FragmentSpread: {
108-
leave: ({ name, directives }) =>
109-
'...' + name + wrap(' ', join(directives, ' ')),
103+
leave: ({ name, arguments: args, directives }) => {
104+
const prefix = '...' + name;
105+
return (
106+
wrappedLineAndArgs(prefix, args) + wrap(' ', join(directives, ' '))
107+
);
108+
},
110109
},
111110

112111
InlineFragment: {
@@ -392,3 +391,15 @@ function hasMultilineItems(maybeArray: Maybe<ReadonlyArray<string>>): boolean {
392391
/* c8 ignore next */
393392
return maybeArray?.some((str) => str.includes('\n')) ?? false;
394393
}
394+
395+
function wrappedLineAndArgs(
396+
prefix: string,
397+
args: ReadonlyArray<string> | undefined,
398+
): string {
399+
let argsLine = prefix + wrap('(', join(args, ', '), ')');
400+
401+
if (argsLine.length > MAX_LINE_LENGTH) {
402+
argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
403+
}
404+
return argsLine;
405+
}

‎src/type/__tests__/introspection-test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,11 @@ describe('Introspection', () => {
859859
isDeprecated: false,
860860
deprecationReason: null,
861861
},
862+
{
863+
name: 'FRAGMENT_VARIABLE_DEFINITION',
864+
isDeprecated: false,
865+
deprecationReason: null,
866+
},
862867
{
863868
name: 'SCHEMA',
864869
isDeprecated: false,

‎src/type/introspection.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,11 @@ export const __DirectiveLocation: GraphQLEnumType = new GraphQLEnumType({
155155
},
156156
VARIABLE_DEFINITION: {
157157
value: DirectiveLocation.VARIABLE_DEFINITION,
158-
description: 'Location adjacent to a variable definition.',
158+
description: 'Location adjacent to an operation variable definition.',
159+
},
160+
FRAGMENT_VARIABLE_DEFINITION: {
161+
value: DirectiveLocation.FRAGMENT_VARIABLE_DEFINITION,
162+
description: 'Location adjacent to a fragment variable definition.',
159163
},
160164
SCHEMA: {
161165
value: DirectiveLocation.SCHEMA,

‎src/utilities/__tests__/printSchema-test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,9 +971,12 @@ describe('Type System Printer', () => {
971971
"""Location adjacent to an inline fragment."""
972972
INLINE_FRAGMENT
973973
974-
"""Location adjacent to a variable definition."""
974+
"""Location adjacent to an operation variable definition."""
975975
VARIABLE_DEFINITION
976976
977+
"""Location adjacent to a fragment variable definition."""
978+
FRAGMENT_VARIABLE_DEFINITION
979+
977980
"""Location adjacent to a schema definition."""
978981
SCHEMA
979982

‎src/utilities/buildASTSchema.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function buildSchema(
9393
): GraphQLSchema {
9494
const document = parse(source, {
9595
noLocation: options?.noLocation,
96-
allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
96+
experimentalFragmentArguments: options?.experimentalFragmentArguments,
9797
});
9898

9999
return buildASTSchema(document, {

0 commit comments

Comments
 (0)