Make FragmentArgumentDefinition a unique AST node to see downstream effects

Author: mjmahone

src/language/__tests__/visitor-test.ts

@@ -477,7 +477,7 @@ describe('Visitor', () => {
       ['enter', 'FragmentDefinition', undefined],
       ['enter', 'Name', 'a'],
       ['leave', 'Name', 'a'],
-      ['enter', 'VariableDefinition', undefined],
+      ['enter', 'FragmentArgumentDefinition', undefined],
       ['enter', 'Variable', undefined],
       ['enter', 'Name', 'v'],
       ['leave', 'Name', 'v'],
@@ -488,7 +488,7 @@ describe('Visitor', () => {
       ['leave', 'NamedType', undefined],
       ['enter', 'BooleanValue', false],
       ['leave', 'BooleanValue', false],
-      ['leave', 'VariableDefinition', undefined],
+      ['leave', 'FragmentArgumentDefinition', undefined],
       ['enter', 'NamedType', undefined],
       ['enter', 'Name', 't'],
       ['leave', 'Name', 't'],

src/language/ast.ts

@@ -149,6 +149,7 @@ export type ASTNode =
   | FragmentSpreadNode
   | InlineFragmentNode
   | FragmentDefinitionNode
+  | FragmentArgumentDefinitionNode
   | IntValueNode
   | FloatValueNode
   | StringValueNode
@@ -227,16 +228,22 @@ export const QueryDocumentKeys: {
   NonNullAssertion: ['nullabilityAssertion'],
   ErrorBoundary: ['nullabilityAssertion'],
 
-  FragmentSpread: ['name', 'directives'],
+  FragmentSpread: ['name', 'arguments', 'directives'],
   InlineFragment: ['typeCondition', 'directives', 'selectionSet'],
   FragmentDefinition: [
     'name',
-    // Note: fragment variable definitions are deprecated and will removed in v17.0.0
-    'variableDefinitions',
+    'arguments',
     'typeCondition',
     'directives',
     'selectionSet',
   ],
+  FragmentArgumentDefinition: [
+    'description',
+    'variable',
+    'type',
+    'defaultValue',
+    'directives',
+  ],
 
   IntValue: [],
   FloatValue: [],
@@ -428,6 +435,7 @@ export interface FragmentSpreadNode {
   readonly kind: Kind.FRAGMENT_SPREAD;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
+  readonly arguments?: ReadonlyArray<ArgumentNode> | undefined;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
 }
 
@@ -443,15 +451,24 @@ export interface FragmentDefinitionNode {
   readonly kind: Kind.FRAGMENT_DEFINITION;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
-  /** @deprecated variableDefinitions will be removed in v17.0.0 */
-  readonly variableDefinitions?:
-    | ReadonlyArray<VariableDefinitionNode>
+  readonly arguments?:
+    | ReadonlyArray<FragmentArgumentDefinitionNode>
     | undefined;
   readonly typeCondition: NamedTypeNode;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
   readonly selectionSet: SelectionSetNode;
 }
 
+export interface FragmentArgumentDefinitionNode {
+  readonly kind: Kind.FRAGMENT_ARGUMENT_DEFINITION;
+  readonly loc?: Location | undefined;
+  readonly description?: StringValueNode | undefined;
+  readonly variable: VariableNode;
+  readonly type: TypeNode;
+  readonly defaultValue?: ConstValueNode | undefined;
+  readonly directives?: ReadonlyArray<ConstDirectiveNode> | undefined;
+}
+
 /** Values */
 
 export type ValueNode =

src/language/kinds.ts

@@ -22,6 +22,7 @@ export enum Kind {
   FRAGMENT_SPREAD = 'FragmentSpread',
   INLINE_FRAGMENT = 'InlineFragment',
   FRAGMENT_DEFINITION = 'FragmentDefinition',
+  FRAGMENT_ARGUMENT_DEFINITION = 'FragmentArgumentDefinition',
 
   /** Values */
   VARIABLE = 'Variable',

src/language/parser.ts

@@ -24,6 +24,7 @@ import type {
   FieldDefinitionNode,
   FieldNode,
   FloatValueNode,
+  FragmentArgumentDefinitionNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   InlineFragmentNode,
@@ -91,23 +92,6 @@ export interface ParseOptions {
    */
   maxTokens?: number | undefined;
 
-  /**
-   * @deprecated will be removed in the v17.0.0
-   *
-   * If enabled, the parser will understand and parse variable definitions
-   * contained in a fragment definition. They'll be represented in the
-   * `variableDefinitions` field of the FragmentDefinitionNode.
-   *
-   * The syntax is identical to normal, query-defined variables. For example:
-   *
-   * ```graphql
-   * fragment A($var: Boolean = false) on T {
-   *   ...
-   * }
-   * ```
-   */
-  allowLegacyFragmentVariables?: boolean | undefined;
-
   /**
    * EXPERIMENTAL:
    *
@@ -550,7 +534,7 @@ export class Parser {
   /**
    * Corresponds to both FragmentSpread and InlineFragment in the spec.
    *
-   * FragmentSpread : ... FragmentName Directives?
+   * FragmentSpread : ... FragmentName Arguments? Directives?
    *
    * InlineFragment : ... TypeCondition? Directives? SelectionSet
    */
@@ -560,9 +544,18 @@ export class Parser {
 
     const hasTypeCondition = this.expectOptionalKeyword('on');
     if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
+      const name = this.parseFragmentName();
+      if (this.peek(TokenKind.PAREN_L)) {
+        return this.node<FragmentSpreadNode>(start, {
+          kind: Kind.FRAGMENT_SPREAD,
+          name,
+          arguments: this.parseArguments(false),
+          directives: this.parseDirectives(false),
+        });
+      }
       return this.node<FragmentSpreadNode>(start, {
         kind: Kind.FRAGMENT_SPREAD,
-        name: this.parseFragmentName(),
+        name,
         directives: this.parseDirectives(false),
       });
     }
@@ -576,29 +569,17 @@ export class Parser {
 
   /**
    * FragmentDefinition :
-   *   - fragment FragmentName on TypeCondition Directives? SelectionSet
+   *   - fragment FragmentName FragmentArgumentsDefinition? on TypeCondition Directives? SelectionSet
    *
    * TypeCondition : NamedType
    */
   parseFragmentDefinition(): FragmentDefinitionNode {
     const start = this._lexer.token;
     this.expectKeyword('fragment');
-    // Legacy support for defining variables within fragments changes
-    // the grammar of FragmentDefinition:
-    //   - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
-    if (this._options.allowLegacyFragmentVariables === true) {
-      return this.node<FragmentDefinitionNode>(start, {
-        kind: Kind.FRAGMENT_DEFINITION,
-        name: this.parseFragmentName(),
-        variableDefinitions: this.parseVariableDefinitions(),
-        typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
-        directives: this.parseDirectives(false),
-        selectionSet: this.parseSelectionSet(),
-      });
-    }
     return this.node<FragmentDefinitionNode>(start, {
       kind: Kind.FRAGMENT_DEFINITION,
       name: this.parseFragmentName(),
+      arguments: this.parseFragmentArgumentDefs(),
       typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
       directives: this.parseDirectives(false),
       selectionSet: this.parseSelectionSet(),
@@ -615,6 +596,48 @@ export class Parser {
     return this.parseName();
   }
 
+  /**
+   * FragmentArgumentsDefinition : ( FragmentArgumentDefinition+ )
+   */
+  parseFragmentArgumentDefs(): Array<FragmentArgumentDefinitionNode> {
+    return this.optionalMany(
+      TokenKind.PAREN_L,
+      this.parseFragmentArgumentDef,
+      TokenKind.PAREN_R,
+    );
+  }
+
+  /**
+   * FragmentArgumentDefinition :
+   *   - Description? Variable : Type DefaultValue? Directives[Const]?
+   *
+   * Note: identical to InputValueDefinition, EXCEPT Name always begins
+   * with $, so we need to parse a Variable out instead of a plain Name.
+   *
+   * Note: identical to VariableDefinition, EXCEPT we allow Description.
+   * Fragments are re-used, and their arguments may need documentation.
+   */
+  parseFragmentArgumentDef(): FragmentArgumentDefinitionNode {
+    const start = this._lexer.token;
+    const description = this.parseDescription();
+    const variable = this.parseVariable();
+    this.expectToken(TokenKind.COLON);
+    const type = this.parseTypeReference();
+    let defaultValue;
+    if (this.expectOptionalToken(TokenKind.EQUALS)) {
+      defaultValue = this.parseConstValueLiteral();
+    }
+    const directives = this.parseConstDirectives();
+    return this.node<FragmentArgumentDefinitionNode>(start, {
+      kind: Kind.FRAGMENT_ARGUMENT_DEFINITION,
+      description,
+      variable,
+      type,
+      defaultValue,
+      directives,
+    });
+  }
+
   // Implements the parsing rules in the Values section.
 
   /**

src/language/printer.ts

@@ -105,8 +105,15 @@ const printDocASTReducer: ASTReducer<string> = {
   // Fragments
 
   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: {
@@ -126,17 +133,30 @@ const printDocASTReducer: ASTReducer<string> = {
     leave: ({
       name,
       typeCondition,
-      variableDefinitions,
+      arguments: args,
       directives,
       selectionSet,
     }) =>
       // Note: fragment variable definitions are experimental and may be changed
       // or removed in the future.
-      `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` +
+      `fragment ${name}${wrap('(', join(args, ', '), ')')} ` +
       `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` +
       selectionSet,
   },
 
+  FragmentArgumentDefinition: {
+    leave: ({ description, variable, type, defaultValue, directives }) =>
+      wrap('', description, '\n') +
+      join(
+        [
+          variable + ': ' + type,
+          wrap('= ', defaultValue),
+          join(directives, ' '),
+        ],
+        ' ',
+      ),
+  },
+
   // Value
 
   IntValue: { leave: ({ value }) => value },

src/utilities/buildASTSchema.ts

@@ -93,7 +93,6 @@ export function buildSchema(
 ): GraphQLSchema {
   const document = parse(source, {
     noLocation: options?.noLocation,
-    allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
   });
 
   return buildASTSchema(document, {