Merge pull request #4373 from GreenGremlin/fragment-variables-fix

Fragment variables fix

CHANGELOG.md

@@ -37,6 +37,14 @@
 - Allow `IntrospectionFragmentMatcher` to match fragments against the root `Query`, as `HeuristicFragmentMatcher` does. <br/>
   [@rynobax](https://github.com/rynobax) in [#4620](https://github.com/apollographql/apollo-client/pull/4620)
 
+### GraphQL Anywhere
+
+- The `graphql` function can now be configured to ignore `@include` and
+  `@skip` directives (useful when walking a fragment to generate prop types
+  or filter result data).  <br/>
+  [@GreenGremlin](https://github.com/GreenGremlin) in [#4373](https://github.com/apollographql/apollo-client/pull/4373)
+
+
 ## Apollo Client 2.5.1
 
 ### apollo-client 2.5.1

package.json

@@ -39,7 +39,7 @@
     {
       "name": "apollo-utilities",
       "path": "./packages/apollo-utilities/lib/bundle.cjs.min.js",
-      "maxSize": "4.2 kB"
+      "maxSize": "4.3 kB"
     }
   ],
   "lint-staged": {
@@ -86,6 +86,7 @@
     "lodash": "4.17.11",
     "pre-commit": "1.2.2",
     "prettier": "1.17.0",
+    "prop-types": "^15.7.2",
     "react": "16.8.6",
     "react-dom": "16.8.6",
     "rollup": "1.10.0",

packages/apollo-utilities/src/directives.ts

@@ -7,6 +7,8 @@ import {
   BooleanValueNode,
   DirectiveNode,
   DocumentNode,
+  ArgumentNode,
+  ValueNode,
 } from 'graphql';
 
 import { visit } from 'graphql/language/visitor';
@@ -40,60 +42,21 @@ export function shouldInclude(
   selection: SelectionNode,
   variables: { [name: string]: any } = {},
 ): boolean {
-  if (!selection.directives) {
-    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;
-    }
-
-    //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
-    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 = directiveArguments[0].value;
+  return getInclusionDirectives(
+    selection.directives,
+  ).every(({ directive, ifArgument }) => {
     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.`,
+        `Invalid variable referenced in @${directive.name.value} directive.`,
       );
     } else {
-      evaledValue = (ifValue as BooleanValueNode).value;
-    }
-
-    if (directiveName === 'skip') {
-      evaledValue = !evaledValue;
-    }
-
-    if (!evaledValue) {
-      res = false;
+      evaledValue = (ifArgument.value as BooleanValueNode).value;
     }
+    return directive.name.value === 'skip' ? !evaledValue : evaledValue;
   });
-
-  return res;
 }
 
 export function getDirectiveNames(doc: DocumentNode) {
@@ -121,3 +84,44 @@ export function hasClientExports(document: DocumentNode) {
     hasDirectives(['export'], document)
   );
 }
+
+export type InclusionDirectives = Array<{
+  directive: DirectiveNode;
+  ifArgument: ArgumentNode;
+}>;
+
+function isInclusionDirective({ name: { value } }: DirectiveNode): boolean {
+  return value === 'skip' || value === 'include';
+}
+
+export function getInclusionDirectives(
+  directives: ReadonlyArray<DirectiveNode>,
+): InclusionDirectives {
+  return directives ? directives.filter(isInclusionDirective).map(directive => {
+    const directiveArguments = directive.arguments;
+    const directiveName = directive.name.value;
+
+    invariant(
+      directiveArguments && 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.`,
+    );
+
+    return { directive, ifArgument };
+  }) : [];
+}
+

packages/graphql-anywhere/package.json

@@ -38,6 +38,7 @@
   "license": "MIT",
   "dependencies": {
     "apollo-utilities": "file:../apollo-utilities",
+    "ts-invariant": "^0.3.2",
     "tslib": "^1.9.3"
   }
 }

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

@@ -0,0 +1,13 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`utilities with a single query can check propTypes for fragments 1`] = `
+Array [
+  "Warning: Failed prop type: alias missing on {}",
+]
+`;
+
+exports[`utilities with a single query can check propTypes for fragments with variables 1`] = `
+Array [
+  "Warning: Failed prop type: height missing on {\\"alias\\":\\"Bob\\",\\"avatar\\":{\\"square\\":\\"abc\\"}}",
+]
+`;

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

@@ -214,7 +214,7 @@ const execute = (graphql, r) => () => {
       }
     `;
 
-    const result = await graphql(resolver, query, null, null, null);
+    const result = await graphql(resolver, query, null, null);
 
     expect(result).toEqual({
       a: {
@@ -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),
       },
     });
   });