graphql · daniel-nagy · Dec 26, 2020
diff --git 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
@@ -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
@@ -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
@@ -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<ArgumentNode>;
   readonly directives?: ReadonlyArray<DirectiveNode>;
 }
 

diff --git 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<ArgumentNode>,
   +directives?: $ReadOnlyArray<DirectiveNode>,
 |};
 

diff --git 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
@@ -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
@@ -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',