Updating propTypes to make fields with variable inclusions optional, …

…when mapPropsToVariables is not passed

packages/apollo-utilities/src/directives.ts

@@ -7,6 +7,8 @@ import {
   BooleanValueNode,
   DirectiveNode,
   DocumentNode,
+  ArgumentNode,
+  ValueNode,
 } from 'graphql';
 
 import { visit } from 'graphql/language/visitor';
@@ -44,56 +46,29 @@ export function shouldInclude(
     return true;
   }
 
-  let res: boolean = true;
-  selection.directives.forEach(directive => {
-    // TODO should move this validation to GraphQL validation once that's implemented.
-    if (directive.name.value !== 'skip' && directive.name.value !== 'include') {
-      // Just don't worry about directives we don't understand
-      return;
-    }
+  const inclusionDirectives: InclusionDirectives = getInclusionDirectives(
+    selection.directives,
+  );
 
-    //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
-    const directiveArguments = directive.arguments || [];
+  if (!inclusionDirectives.length) {
+    return true;
+  }
+  return inclusionDirectives.every(({ directive, ifArgument }) => {
     const directiveName = directive.name.value;
 
-    invariant(
-      directiveArguments.length === 1,
-      `Incorrect number of arguments for the @${directiveName} directive.`,
-    );
-
-    const ifArgument = directiveArguments[0];
-    invariant(
-      ifArgument.name && ifArgument.name.value === 'if',
-      `Invalid argument for the @${directiveName} directive.`,
-    );
-
-    const ifValue = directiveArguments[0].value;
     let evaledValue: boolean = false;
-    if (!ifValue || ifValue.kind !== 'BooleanValue') {
-      // means it has to be a variable value if this is a valid @skip or @include directive
-      invariant(
-        ifValue.kind === 'Variable',
-        `Argument for the @${directiveName} directive must be a variable or a boolean value.`,
-      );
-      evaledValue = variables[(ifValue as VariableNode).name.value];
+    if (ifArgument.value.kind === 'Variable') {
+      evaledValue = variables[(ifArgument.value as VariableNode).name.value];
       invariant(
         evaledValue !== void 0,
         `Invalid variable referenced in @${directiveName} directive.`,
       );
     } else {
-      evaledValue = (ifValue as BooleanValueNode).value;
+      evaledValue = (ifArgument.value as BooleanValueNode).value;
     }
 
-    if (directiveName === 'skip') {
-      evaledValue = !evaledValue;
-    }
-
-    if (!evaledValue) {
-      res = false;
-    }
+    return directiveName === 'skip' ? !evaledValue : evaledValue;
   });
-
-  return res;
 }
 
 export function getDirectiveNames(doc: DocumentNode) {
@@ -121,3 +96,51 @@ export function hasClientExports(document: DocumentNode) {
     hasDirectives(['export'], document)
   );
 }
+
+export type InclusionDirectives = Array<{
+  directive: DirectiveNode;
+  ifArgument: ArgumentNode;
+}>;
+
+export function getInclusionDirectives(
+  directives: ReadonlyArray<DirectiveNode>,
+): InclusionDirectives {
+  const result: InclusionDirectives = [];
+  if (!directives || !directives.length) {
+    return result;
+  }
+  directives.forEach(directive => {
+    if (!isInclusionDirective(directive)) {
+      return;
+    }
+    const directiveArguments = directive.arguments || [];
+    const directiveName = directive.name.value;
+
+    invariant(
+      directiveArguments.length === 1,
+      `Incorrect number of arguments for the @${directiveName} directive.`,
+    );
+
+    const ifArgument = directiveArguments[0];
+    invariant(
+      ifArgument.name && ifArgument.name.value === 'if',
+      `Invalid argument for the @${directiveName} directive.`,
+    );
+
+    const ifValue: ValueNode = ifArgument.value;
+
+    // means it has to be a variable value if this is a valid @skip or @include directive
+    invariant(
+      ifValue &&
+        (ifValue.kind === 'Variable' || ifValue.kind === 'BooleanValue'),
+      `Argument for the @${directiveName} directive must be a variable or a boolean value.`,
+    );
+    result.push({ directive, ifArgument });
+  });
+
+  return result;
+}
+
+export function isInclusionDirective(directive: DirectiveNode): boolean {
+  return directive.name.value === 'skip' || directive.name.value === 'include';
+}

packages/graphql-anywhere/src/__tests__/__snapshots__/utilities.ts.snap

@@ -8,6 +8,6 @@ Array [
 
 exports[`utilities with a single query can check propTypes for fragments with variables 1`] = `
 Array [
-  "Warning: Failed prop type: avatar missing on {\\"alias\\":\\"Bob\\",\\"height\\":1.89}",
+  "Warning: Failed prop type: height missing on {\\"alias\\":\\"Bob\\",\\"avatar\\":{\\"square\\":\\"abc\\"}}",
 ]
 `;

packages/graphql-anywhere/src/__tests__/index.ts

@@ -423,7 +423,7 @@ const execute = (graphql, r) => () => {
     });
   });
 
-  it('passes info including isLeaf, resultKey and directives', async () => {
+  it('passes info including isLeaf, resultKey, directives, and field', async () => {
     const leafMap: { [s: string]: ExecInfo } = {};
 
     const resolver: Resolver = (fieldName, root, args, context, info) => {
@@ -450,6 +450,8 @@ const execute = (graphql, r) => () => {
         isLeaf: false,
 
         resultKey: 'alias',
+
+        field: expect.any(Object),
       },
 
       b: {
@@ -458,6 +460,8 @@ const execute = (graphql, r) => () => {
         isLeaf: true,
 
         resultKey: 'b',
+
+        field: expect.any(Object),
       },
 
       hasDirective: {
@@ -470,6 +474,8 @@ const execute = (graphql, r) => () => {
         isLeaf: true,
 
         resultKey: 'hasDirective',
+
+        field: expect.any(Object),
       },
     });
   });

packages/graphql-anywhere/src/__tests__/utilities.ts

@@ -1,5 +1,5 @@
 import gql, { disableFragmentWarnings } from 'graphql-tag';
-import {checkPropTypes} from 'prop-types';
+import { checkPropTypes } from 'prop-types';
 
 // Turn off warnings for repeated fragment names
 disableFragmentWarnings();
@@ -90,15 +90,14 @@ describe('utilities', () => {
         },
       },
     ];
-    let consoleSpy;
 
     beforeEach(() => {
       checkPropTypes.resetWarningCache();
-      consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+      jest.spyOn(global.console, 'error').mockImplementation(() => {});
     });
 
     afterEach(() => {
-      consoleSpy.mockRestore();
+      global.console.error.mockRestore();
     });
 
     it('can filter data', () => {
@@ -128,26 +127,37 @@ describe('utilities', () => {
         foo: propType(fragment),
       };
       checkPropTypes(propTypes, filteredData, 'prop', 'MyComponent');
-      expect(consoleSpy).not.toHaveBeenCalled();
+      expect(global.console.error).not.toHaveBeenCalled();
       checkPropTypes(propTypes, { foo: {} }, 'prop', 'MyComponent');
-      expect(consoleSpy.mock.calls[0]).toMatchSnapshot();
+      expect(global.console.error.mock.calls[0]).toMatchSnapshot();
     });
 
     it('can generate propTypes for fragments with variables', () => {
       expect(propType(fragmentWithAVariable)).toEqual(expect.any(Function));
     });
 
     it('can check propTypes for fragments with variables', () => {
-      const mapPropsToVariables = () => ({foo: true});
+      const mapPropsToVariables = () => null;
       const propTypes = {
         foo: propType(fragmentWithAVariable, mapPropsToVariables),
       };
-      checkPropTypes(propTypes, filteredData, 'prop', 'MyComponent');
-      expect(consoleSpy).not.toHaveBeenCalled();
+      checkPropTypes(propTypes, { foo: filteredData }, 'prop', 'MyComponent');
+      expect(global.console.error).not.toHaveBeenCalled();
       const badProps = { foo: { ...filteredData } };
-      delete badProps.foo.avatar;
+      delete badProps.foo.height;
       checkPropTypes(propTypes, badProps, 'prop', 'MyComponent');
-      expect(consoleSpy.mock.calls[0]).toMatchSnapshot();
+      expect(global.console.error).toHaveBeenCalled();
+      expect(global.console.error.mock.calls[0]).toMatchSnapshot();
+    });
+
+    it('makes variable inclusion props optional, when no variables are passed', () => {
+      const propTypes = {
+        foo: propType(fragmentWithAVariable),
+      };
+      const propsWithoutAvatar = { foo: { ...filteredData } };
+      delete propsWithoutAvatar.foo.avatar;
+      checkPropTypes(propTypes, propsWithoutAvatar, 'prop', 'MyComponent');
+      expect(global.console.error).not.toHaveBeenCalled();
     });
 
     it('can check matching data', () => {

packages/graphql-anywhere/src/async.ts

@@ -157,6 +157,7 @@ async function executeField(
     isLeaf: !field.selectionSet,
     resultKey: resultKeyNameFromField(field),
     directives: getDirectiveInfoFromField(field, variables),
+    field,
   };
 
   const result = await resolver(fieldName, rootValue, args, contextValue, info);

packages/graphql-anywhere/src/graphql.ts

@@ -53,6 +53,7 @@ export type ExecInfo = {
   isLeaf: boolean;
   resultKey: string;
   directives: DirectiveInfo;
+  field: FieldNode;
 };
 
 export type ExecOptions = {
@@ -186,6 +187,7 @@ function executeField(
     isLeaf: !field.selectionSet,
     resultKey: resultKeyNameFromField(field),
     directives: getDirectiveInfoFromField(field, variables),
+    field,
   };
 
   const result = resolver(fieldName, rootValue, args, contextValue, info);