From 45905198bc1dc0f16a81315b018bb95ad6319530 Mon Sep 17 00:00:00 2001 From: Daniel Nagy Date: Sat, 26 Dec 2020 10:54:37 -0800 Subject: [PATCH] Add fragment spread arguments syntax --- src/language/__tests__/parser-test.js | 122 +++++++++++++++++++++++++ src/language/__tests__/printer-test.js | 43 +++++++++ src/language/__tests__/visitor-test.js | 47 ++++++++++ src/language/ast.d.ts | 2 + src/language/ast.js | 2 + src/language/parser.js | 6 ++ src/language/printer.js | 13 ++- src/language/visitor.js | 7 +- 8 files changed, 239 insertions(+), 3 deletions(-) diff --git a/src/language/__tests__/parser-test.js b/src/language/__tests__/parser-test.js index 65826f4ef8..dced0028f2 100644 --- a/src/language/__tests__/parser-test.js +++ b/src/language/__tests__/parser-test.js @@ -372,6 +372,128 @@ describe('Parser', () => { expect(() => parse(document)).to.throw('Syntax Error'); }); + it('Legacy: allows parsing fragment spread arguments if `allowLegacyFragmentVariables` is enabled', () => { + const fragmentSpread = '{ ...Foo(v: $v) }'; + + expect(() => + parse(fragmentSpread, { allowLegacyFragmentVariables: true }), + ).to.not.throw(); + expect(() => parse(fragmentSpread)).to.throw('Syntax Error'); + }); + + it('Legacy: does not parse arguments on inline fragments', () => { + const inlineFragment = '{ ... on Foo(v: $v) { id } }'; + expect(() => + parse(inlineFragment, { allowLegacyFragmentVariables: true }), + ).to.throw('Syntax Error'); + }); + + it('Legacy: parses fragment spread arguments', () => { + const document = parse('{ ...Foo(v: $v) }', { + allowLegacyFragmentVariables: true, + }); + + expect(toJSONDeep(document)).to.have.deep.nested.property( + 'definitions[0].selectionSet.selections[0]', + { + arguments: [ + { + kind: Kind.ARGUMENT, + loc: { end: 14, start: 9 }, + name: { + kind: Kind.NAME, + loc: { end: 10, start: 9 }, + value: 'v', + }, + value: { + kind: Kind.VARIABLE, + loc: { end: 14, start: 12 }, + name: { + kind: Kind.NAME, + loc: { end: 14, start: 13 }, + value: 'v', + }, + }, + }, + ], + directives: [], + kind: Kind.FRAGMENT_SPREAD, + loc: { end: 15, start: 2 }, + name: { + kind: Kind.NAME, + loc: { end: 8, start: 5 }, + value: 'Foo', + }, + }, + ); + }); + + it('Legacy: parses fragment spread arguments and directives', () => { + const document = parse('{ ...Foo(v: $v) @skip(if: true) }', { + allowLegacyFragmentVariables: true, + }); + + expect(toJSONDeep(document)).to.have.deep.nested.property( + 'definitions[0].selectionSet.selections[0]', + { + arguments: [ + { + kind: Kind.ARGUMENT, + loc: { end: 14, start: 9 }, + name: { + kind: Kind.NAME, + loc: { end: 10, start: 9 }, + value: 'v', + }, + value: { + kind: Kind.VARIABLE, + loc: { end: 14, start: 12 }, + name: { + kind: Kind.NAME, + loc: { end: 14, start: 13 }, + value: 'v', + }, + }, + }, + ], + directives: [ + { + arguments: [ + { + kind: Kind.ARGUMENT, + loc: { end: 30, start: 22 }, + name: { + kind: Kind.NAME, + loc: { end: 24, start: 22 }, + value: 'if', + }, + value: { + kind: Kind.BOOLEAN, + loc: { end: 30, start: 26 }, + value: true, + }, + }, + ], + kind: Kind.DIRECTIVE, + loc: { end: 31, start: 16 }, + name: { + kind: Kind.NAME, + loc: { end: 21, start: 17 }, + value: 'skip', + }, + }, + ], + kind: Kind.FRAGMENT_SPREAD, + loc: { end: 31, start: 2 }, + name: { + kind: Kind.NAME, + loc: { end: 8, start: 5 }, + value: 'Foo', + }, + }, + ); + }); + it('contains location information that only stringifies start/end', () => { const result = parse('{ id }'); diff --git a/src/language/__tests__/printer-test.js b/src/language/__tests__/printer-test.js index 29c653dfb2..c1a7b4f769 100644 --- a/src/language/__tests__/printer-test.js +++ b/src/language/__tests__/printer-test.js @@ -145,6 +145,49 @@ describe('Printer: Query document', () => { `); }); + it('Legacy: correctly prints fragment spread arguments', () => { + const parsed = parse('{ ...Foo(bar: $bar) }', { + allowLegacyFragmentVariables: true, + }); + + expect(print(parsed)).to.equal(dedent` + { + ...Foo(bar: $bar) + } + `); + }); + + it('Legacy: correctly prints fragment spread arguments that exceed the max line length', () => { + const parsed = parse( + '{ ...Dogs(breads: ["Boston Terrier", "Poodle", "Beagle"], likesPets: true, likesToys: true) }', + { + allowLegacyFragmentVariables: true, + }, + ); + + expect(print(parsed)).to.equal(dedent` + { + ...Dogs( + breads: ["Boston Terrier", "Poodle", "Beagle"] + likesPets: true + likesToys: true + ) + } + `); + }); + + it('Legacy: correctly prints fragment spread arguments with directives', () => { + const parsed = parse('{ ...Foo(bar: $bar) @skip(if: $isBaz) }', { + allowLegacyFragmentVariables: true, + }); + + expect(print(parsed)).to.equal(dedent` + { + ...Foo(bar: $bar) @skip(if: $isBaz) + } + `); + }); + it('prints kitchen sink', () => { const printed = print(parse(kitchenSinkQuery)); diff --git a/src/language/__tests__/visitor-test.js b/src/language/__tests__/visitor-test.js index bfe01c7719..ce06c3d203 100644 --- a/src/language/__tests__/visitor-test.js +++ b/src/language/__tests__/visitor-test.js @@ -470,6 +470,53 @@ describe('Visitor', () => { ]); }); + it('Legacy: visits fragment spread arguments', () => { + const ast = parse('{ ...Foo(v: $v) }', { + noLocation: true, + allowLegacyFragmentVariables: true, + }); + + const visited = []; + + visit(ast, { + enter(node) { + checkVisitorFnArgs(ast, arguments); + visited.push(['enter', node.kind, getValue(node)]); + }, + leave(node) { + checkVisitorFnArgs(ast, arguments); + visited.push(['leave', node.kind, getValue(node)]); + }, + }); + + const argumentNode = { + kind: Kind.VARIABLE, + name: { kind: Kind.NAME, loc: undefined, value: 'v' }, + loc: undefined, + }; + + expect(visited).to.deep.equal([ + ['enter', Kind.DOCUMENT, undefined], + ['enter', Kind.OPERATION_DEFINITION, undefined], + ['enter', Kind.SELECTION_SET, undefined], + ['enter', Kind.FRAGMENT_SPREAD, undefined], + ['enter', Kind.NAME, 'Foo'], + ['leave', Kind.NAME, 'Foo'], + ['enter', Kind.ARGUMENT, argumentNode], + ['enter', Kind.NAME, 'v'], + ['leave', Kind.NAME, 'v'], + ['enter', Kind.VARIABLE, undefined], + ['enter', Kind.NAME, 'v'], + ['leave', Kind.NAME, 'v'], + ['leave', Kind.VARIABLE, undefined], + ['leave', Kind.ARGUMENT, argumentNode], + ['leave', Kind.FRAGMENT_SPREAD, undefined], + ['leave', Kind.SELECTION_SET, undefined], + ['leave', Kind.OPERATION_DEFINITION, undefined], + ['leave', Kind.DOCUMENT, undefined], + ]); + }); + it('visits kitchen sink', () => { const ast = parse(kitchenSinkQuery); const visited = []; diff --git a/src/language/ast.d.ts b/src/language/ast.d.ts index 693657d642..766ab7fd2c 100644 --- a/src/language/ast.d.ts +++ b/src/language/ast.d.ts @@ -282,6 +282,8 @@ export interface FragmentSpreadNode { readonly kind: 'FragmentSpread'; readonly loc?: Location; readonly name: NameNode; + // Note: fragment variable definitions are deprecated and will removed in v17.0.0 + readonly arguments?: ReadonlyArray; readonly directives?: ReadonlyArray; } diff --git a/src/language/ast.js b/src/language/ast.js index c0a9a615e7..6954cf0e44 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -308,6 +308,8 @@ export type FragmentSpreadNode = {| +kind: 'FragmentSpread', +loc?: Location, +name: NameNode, + // Note: fragment variable definitions are deprecated and will removed in v17.0.0 + +arguments?: $ReadOnlyArray, +directives?: $ReadOnlyArray, |}; diff --git a/src/language/parser.js b/src/language/parser.js index f5f0a876b1..135705c719 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -434,6 +434,12 @@ export class Parser { return { kind: Kind.FRAGMENT_SPREAD, name: this.parseFragmentName(), + // Legacy support for fragment variables changes the grammar of a + // FragmentSpreadNode. + arguments: + this._options?.allowLegacyFragmentVariables ?? false + ? this.parseArguments(false) + : undefined, directives: this.parseDirectives(false), loc: this.loc(start), }; diff --git a/src/language/printer.js b/src/language/printer.js index 334742d967..6f924ad3a1 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -69,9 +69,18 @@ const printDocASTReducer: any = { // Fragments + // Note: fragment variable definitions are deprecated and will removed in v17.0.0 FragmentSpread: { - leave: ({ name, directives }) => - '...' + name + wrap(' ', join(directives, ' ')), + leave: ({ name, arguments: args, directives }) => { + const prefix = '...' + name; + let argsLine = prefix + wrap('(', join(args, ', '), ')'); + + if (argsLine.length > MAX_LINE_LENGTH) { + argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); + } + + return argsLine + wrap(' ', join(directives, ' ')); + }, }, InlineFragment: { diff --git a/src/language/visitor.js b/src/language/visitor.js index 582aaf4423..1b22453e5e 100644 --- a/src/language/visitor.js +++ b/src/language/visitor.js @@ -54,7 +54,12 @@ const QueryDocumentKeys = { Field: ['alias', 'name', 'arguments', 'directives', 'selectionSet'], Argument: ['name', 'value'], - FragmentSpread: ['name', 'directives'], + FragmentSpread: [ + 'name', + // Note: fragment variable definitions are deprecated and will removed in v17.0.0 + 'arguments', + 'directives', + ], InlineFragment: ['typeCondition', 'directives', 'selectionSet'], FragmentDefinition: [ 'name',