Fragment Args rebased for 2022

Author: mjmahone

src/execution/__tests__/variables-test.ts

@@ -1006,6 +1006,166 @@ describe('Execute: Handles inputs', () => {
     });
   });
 
+  describe('using fragment arguments', () => {
+    it('when there are no fragment arguments', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+
+        fragment a on TestType {
+          fieldWithNonNullableStringInput(input: "A")
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+
+        fragment a($value: String!) on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+
+        fragment a($value: String!) on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: null,
+        },
+      });
+    });
+
+    it('when the definition has a default and is provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when the definition has a default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"B"',
+        },
+      });
+    });
+
+    it('when the definition has a non-nullable default and is provided null', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: null)
+        }
+
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: 'null',
+        },
+      });
+    });
+
+    it('when the definition has no default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+
+        fragment a($value: String) on TestType {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            '"Hello World"',
+        },
+      });
+    });
+
+    it('when the argument variable is nested in a complex type', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "C")
+        }
+
+        fragment a($value: String) on TestType {
+          list(input: ["A", "B", $value, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+
+    it('when argument variables are used recursively', () => {
+      const result = executeQuery(`
+        query {
+          ...a(aValue: "C")
+        }
+
+        fragment a($aValue: String) on TestType {
+          ...b(bValue: $aValue)
+        }
+
+        fragment b($bValue: String) on TestType {
+          list(input: ["A", "B", $bValue, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+  });
+
   describe('getVariableValues: limit maximum number of coercion errors', () => {
     const doc = parse(`
       query ($input: [String!]) {

src/execution/collectFields.ts

@@ -20,6 +20,8 @@ import type { GraphQLSchema } from '../type/schema';
 
 import { typeFromAST } from '../utilities/typeFromAST';
 
+import { visit } from '../language/visitor';
+
 import { getDirectiveValues } from './values';
 
 /**
@@ -144,7 +146,7 @@ function collectFieldsImpl(
           fragments,
           variableValues,
           runtimeType,
-          fragment.selectionSet,
+          selectionSetWithArgumentsApplied,
           fields,
           visitedFragmentNames,
         );
@@ -206,3 +208,44 @@ function doesFragmentConditionMatch(
 function getFieldEntryKey(node: FieldNode): string {
   return node.alias ? node.alias.value : node.name.value;
 }
+
+/**
+ *
+ * When a fragment spread is provided with arguments,
+ * visit that fragment's definition and replace those arguments'
+ * variable usages with the provided argument value.
+ */
+function selectionSetWithFragmentArgumentsApplied(
+  fragment: FragmentDefinitionNode,
+  fragmentSpread: FragmentSpreadNode,
+): SelectionSetNode {
+  const variableDefinitions = fragment.variableDefinitions;
+  if (!variableDefinitions) {
+    return fragment.selectionSet;
+  }
+
+  const providedArguments: Map<string, ArgumentNode> = new Map();
+  if (fragmentSpread.arguments) {
+    for (const argument of fragmentSpread.arguments) {
+      providedArguments.set(argument.name.value, argument);
+    }
+  }
+
+  const fragmentVariableValues: Map<string, ValueNode> = new Map();
+  for (const variableDef of variableDefinitions) {
+    const variableName = variableDef.variable.name.value;
+    const providedArg = providedArguments.get(variableName);
+    if (providedArg) {
+      fragmentVariableValues.set(variableName, providedArg.value);
+    } else if (variableDef.defaultValue) {
+      fragmentVariableValues.set(variableName, variableDef.defaultValue);
+    }
+    // Otherwise just preserve the variable as-is: it will be treated as unset by the executor.
+  }
+
+  return visit(fragment.selectionSet, {
+    Variable(node) {
+      return fragmentVariableValues.get(node.name.value);
+    },
+  });
+}

src/language/__tests__/parser-test.ts

@@ -594,19 +594,13 @@ describe('Parser', () => {
   it('allows parsing fragment defined variables', () => {
     const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
 
-    expect(() =>
-      parse(document, { allowFragmentArguments: true }),
-    ).to.not.throw();
-    expect(() => parse(document)).to.throw('Syntax Error');
+    expect(() => parse(document)).to.not.throw();
   });
 
   it('allows parsing fragment spread arguments', () => {
     const document = 'fragment a on t { ...b(v: $v) }';
 
-    expect(() =>
-      parse(document, { allowFragmentArguments: true }),
-    ).to.not.throw();
-    expect(() => parse(document)).to.throw('Syntax Error');
+    expect(() => parse(document)).to.not.throw();
   });
 
   it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {

src/language/__tests__/printer-test.ts

@@ -113,7 +113,6 @@ describe('Printer: Query document', () => {
   it('prints fragment with variable directives', () => {
     const queryASTWithVariableDirective = parse(
       'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
-      { allowFragmentArguments: true },
     );
     expect(print(queryASTWithVariableDirective)).to.equal(dedent`
       fragment Foo($foo: TestType @test) on TestType @testDirective {
@@ -129,7 +128,6 @@ describe('Printer: Query document', () => {
           id
         }
       `,
-      { allowFragmentArguments: true },
     );
     expect(print(fragmentWithVariable)).to.equal(dedent`
       fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
@@ -141,7 +139,6 @@ describe('Printer: Query document', () => {
   it('prints fragment spread with arguments', () => {
     const queryASTWithVariableDirective = parse(
       'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
-      { allowFragmentArguments: true },
     );
     expect(print(queryASTWithVariableDirective)).to.equal(dedent`
       fragment Foo on TestType {
@@ -153,7 +150,6 @@ describe('Printer: Query document', () => {
   it('prints fragment spread with multi-line arguments', () => {
     const queryASTWithVariableDirective = parse(
       '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") }',
-      { allowFragmentArguments: true },
     );
     expect(print(queryASTWithVariableDirective)).to.equal(dedent`
       fragment Foo on TestType {

src/language/__tests__/visitor-test.ts

@@ -458,7 +458,6 @@ describe('Visitor', () => {
   it('visits variables defined in fragments', () => {
     const ast = parse('fragment a($v: Boolean = false) on t { f }', {
       noLocation: true,
-      allowFragmentArguments: true,
     });
     const visited: Array<any> = [];
 

src/language/parser.ts

@@ -80,6 +80,8 @@ export interface ParseOptions {
    * in the source that they correspond to. This configuration flag
    * disables that behavior for performance or testing.
    */
+<<<<<<< HEAD
+<<<<<<< HEAD
   noLocation?: boolean | undefined;
 
   /**
@@ -123,6 +125,12 @@ export interface ParseOptions {
    * future.
    */
   experimentalClientControlledNullability?: boolean | undefined;
+=======
+  noLocation?: boolean;
+>>>>>>> Remove flag for parsing, but add default validation to indicate fragment args are not yet spec-supported, and may be missing validation.
+=======
+  noLocation?: boolean;
+>>>>>>> 01414be319c245415c1c5d4818a4fe9179c2909d
 }
 
 /**
@@ -551,17 +559,18 @@ export class Parser {
 
     const hasTypeCondition = this.expectOptionalKeyword('on');
     if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
-      if (this._options?.allowFragmentArguments === true) {
+      const name = this.parseFragmentName();
+      if (this.peek(TokenKind.PAREN_L)) {
         return this.node<FragmentSpreadNode>(start, {
           kind: Kind.FRAGMENT_SPREAD,
-          name: this.parseFragmentName(),
+          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),
       });
     }
@@ -588,7 +597,7 @@ export class Parser {
     if (this._options.allowLegacyFragmentVariables === true) {
       return this.node<FragmentDefinitionNode>(start, {
         kind: Kind.FRAGMENT_DEFINITION,
-        name: this.parseFragmentName(),
+        name,
         variableDefinitions: this.parseVariableDefinitions(),
         typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
         directives: this.parseDirectives(false),
@@ -597,7 +606,7 @@ export class Parser {
     }
     return this.node<FragmentDefinitionNode>(start, {
       kind: Kind.FRAGMENT_DEFINITION,
-      name: this.parseFragmentName(),
+      name,
       typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
       directives: this.parseDirectives(false),
       selectionSet: this.parseSelectionSet(),

src/utilities/buildASTSchema.ts

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