From d89d14dda2ebc41eefdca2db28eace4baebd8acb Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 9 Jun 2016 17:46:04 -0700
Subject: [PATCH 01/17] added utility functions to add in skip directives and
 unit tests for the same

---
 src/queries/directives.ts | 136 +++++++++++++++++++++++++++++++++++
 test/directives.ts        | 145 ++++++++++++++++++++++++++++++++++++++
 test/tests.ts             |   1 +
 3 files changed, 282 insertions(+)
 create mode 100644 src/queries/directives.ts
 create mode 100644 test/directives.ts

diff --git a/src/queries/directives.ts b/src/queries/directives.ts
new file mode 100644
index 00000000000..7a1cc0b3583
--- /dev/null
+++ b/src/queries/directives.ts
@@ -0,0 +1,136 @@
+// Provides the methods that allow QueryManager.fetchQuery to handle
+// the `skip` and `include` directives within GraphQL.
+
+import {
+  SelectionSet,
+  Directive,
+  Selection,
+  Document,
+  InlineFragment,
+  Field,
+  BooleanValue,
+  Variable,
+} from 'graphql';
+
+import {
+  getQueryDefinition,
+  getFragmentDefinitions,
+} from './getFromAST';
+
+import identity = require('lodash.identity');
+import cloneDeep = require('lodash.clonedeep');
+
+// A handler that takes a selection, variables, and a directive to apply to that selection.
+export type DirectiveResolver = (selection: Selection,
+                                 variables: { [name: string]: any },
+                                 directive: Directive) => Selection
+
+export function applyDirectives(doc: Document,
+                                variables?: { [name: string]: any },
+                                directiveResolvers?: { [name: string]: DirectiveResolver} )
+: Document {
+  if (!variables) {
+    variables = {};
+  }
+  if (!directiveResolvers) {
+    directiveResolvers = {};
+  }
+
+  const newDoc = cloneDeep(doc);
+  const fragmentDefs = getFragmentDefinitions(newDoc);
+  const queryDef = getQueryDefinition(newDoc);
+  const newSelSet = applyDirectivesToSelectionSet(queryDef.selectionSet,
+                                                  variables,
+                                                  directiveResolvers);
+  queryDef.selectionSet = newSelSet;
+  newDoc.definitions = fragmentDefs.map((fragmentDef) => {
+    const fragmentSelSet = applyDirectivesToSelectionSet(fragmentDef.selectionSet,
+                                                         variables,
+                                                         directiveResolvers);
+    fragmentDef.selectionSet = fragmentSelSet;
+    return fragmentDef;
+  });
+  newDoc.definitions.unshift(queryDef);
+
+  return newDoc;
+}
+
+export function applyDirectivesToSelectionSet(selSet: SelectionSet,
+                                              variables: { [name: string]: any },
+                                              directiveResolvers: {
+                                                [name: string]: DirectiveResolver})
+: SelectionSet {
+
+  const selections = selSet.selections;
+  selSet.selections = selections.map((selection) => {
+    let newSelection: Selection = selection;
+    selection.directives.forEach((directive) => {
+      if (directive.name && directive.name.value) {
+        const directiveResolver = directiveResolvers[directive.name.value];
+        newSelection = directiveResolver(newSelection, variables, directive);
+      }
+    });
+
+    if (newSelection !== undefined) {
+      const withSelSet = selection as (InlineFragment | Field);
+      // recursively operate on selection sets within this selection set.
+      if (withSelSet.kind === 'InlineFragment' ||
+          (withSelSet.kind === 'Field' && withSelSet.selectionSet)) {
+        withSelSet.selectionSet = applyDirectivesToSelectionSet(withSelSet.selectionSet,
+                                                               variables,
+                                                               directiveResolvers);
+      }
+      return newSelection;
+    }
+  });
+
+  //filter out undefined values
+  selSet.selections = selSet.selections.filter(identity);
+  return selSet;
+}
+
+export function skipDirectiveResolver(selection: Selection,
+                                      variables: { [name: string]: any },
+                                      directive: Directive)
+: Selection {
+  //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
+  const directiveArguments = directive.arguments;
+  if (directiveArguments.length !== 1) {
+    throw new Error('Incorrect number of arguments for the @skip directive.');
+  }
+
+  const ifArgument = directive.arguments[0];
+  if (!ifArgument.name || ifArgument.name.value !== 'if') {
+    throw new Error('Invalid argument for the @skip directive.');
+  }
+
+  const ifValue = directive.arguments[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 directive
+    if (ifValue.kind !== 'Variable') {
+      throw new Error('Invalid argument value for the @skip directive.');
+    } else {
+      evaledValue = variables[(ifValue as Variable).name.value];
+      if (evaledValue === undefined) {
+        throw new Error('Invalid variable %s referenced in @skip directive.');
+      }
+    }
+  } else {
+    evaledValue = (ifValue as BooleanValue).value;
+  }
+
+  // if the value is false, then don't skip it.
+  if (!evaledValue) {
+    return selection;
+  } else {
+    return undefined;
+  }
+}
+
+export function applySkipResolver(doc: Document, variables?: { [name: string]: any })
+: Document {
+  return applyDirectives(doc, variables, {
+    'skip': skipDirectiveResolver,
+  });
+}
diff --git a/test/directives.ts b/test/directives.ts
new file mode 100644
index 00000000000..ae5215e5cde
--- /dev/null
+++ b/test/directives.ts
@@ -0,0 +1,145 @@
+import * as chai from 'chai';
+const { assert } = chai;
+
+import {
+  applySkipResolver,
+} from '../src/queries/directives';
+
+import gql from '../src/gql';
+
+import { print } from 'graphql/language/printer';
+
+describe('query directives', () => {
+  it('should trim skipped fields from a selection set', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: true)
+        }
+      }`;
+    const expQuery = gql`
+      query {
+        author {
+          firstName
+        }
+      }`;
+
+    const newQuery = applySkipResolver(query);
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should trimmed skipped fields in queries with inlined fragments', () => {
+    const query = gql`
+      query {
+        author {
+          ...authorDetails on Author {
+            firstName
+            lastName @skip(if: true)
+          }
+        }
+      }`;
+    const expQuery = gql`
+      query {
+        author {
+          ...authorDetails on Author {
+            firstName
+          }
+        }
+      }`;
+    const newQuery = applySkipResolver(query);
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should throw an error if the argument to skip is not present', () => {
+    const query = gql`
+      query {
+        author {
+          ...authorDetails on Author {
+            firstName
+            lastName @skip(someotherag: true)
+          }
+        }
+     }`;
+    assert.throws(() => {
+      applySkipResolver(query);
+    });
+  });
+
+  it('should throw an error if there are extra arguments to skip', () => {
+    const query = gql`
+      query {
+        author {
+          ... on Author {
+            firstName
+            lastName @skip(if: true, useless: false)
+          }
+        }
+      }`;
+    assert.throws(() => {
+      applySkipResolver(query);
+    });
+  });
+
+  it('should skip stuff inside fragment spreads', () => {
+    const query = gql`
+      query {
+        author {
+          ...authorDetails
+        }
+      }
+      fragment authorDetails on Author {
+        firstName
+        lastName @skip(if: true)
+      }`;
+
+    const expQuery = gql`
+      query {
+        author {
+          ...authorDetails
+        }
+      }
+      fragment authorDetails on Author {
+         firstName
+      }`;
+
+    const newQuery = applySkipResolver(query);
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should evaluate variables for skips', () => {
+    const query = gql`
+      query myQuery($shouldSkip: Boolean) {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip)
+        }
+      }`;
+    const variables = {
+      shouldSkip: true,
+    };
+    const expQuery = gql`
+      query myQuery($shouldSkip: Boolean) {
+        author {
+          firstName
+        }
+      }`;
+
+    const newQuery = applySkipResolver(query, variables);
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should throw an error if an invalid variable is referenced in the skip directive', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip)
+        }
+      }`;
+    const variables = {};
+    assert.throws(() => {
+      applySkipResolver(query, variables);
+    });
+  });
+});
diff --git a/test/tests.ts b/test/tests.ts
index c264c347bd3..fe168c262d4 100644
--- a/test/tests.ts
+++ b/test/tests.ts
@@ -19,3 +19,4 @@ import './store';
 import './gql';
 import './queryTransform';
 import './getFromAST';
+import './directives';

From 948618d3ec7b64477744ef8227f55d96136d6d2c Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Fri, 10 Jun 2016 15:01:46 -0700
Subject: [PATCH 02/17] added  utility functions

---
 src/queries/directives.ts |  59 ++++++++++++---
 test/directives.ts        | 155 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 204 insertions(+), 10 deletions(-)

diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 7a1cc0b3583..295b218f6d2 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -1,4 +1,4 @@
-// Provides the methods that allow QueryManager.fetchQuery to handle
+// Provides the methods that allow QueryManager to handle
 // the `skip` and `include` directives within GraphQL.
 
 import {
@@ -63,11 +63,29 @@ export function applyDirectivesToSelectionSet(selSet: SelectionSet,
 
   const selections = selSet.selections;
   selSet.selections = selections.map((selection) => {
+
     let newSelection: Selection = selection;
+    let currSelection: Selection = selection;
+    let toBeRemoved = null;
+
     selection.directives.forEach((directive) => {
       if (directive.name && directive.name.value) {
         const directiveResolver = directiveResolvers[directive.name.value];
-        newSelection = directiveResolver(newSelection, variables, directive);
+        newSelection = directiveResolver(currSelection, variables, directive);
+
+        // add handling for the case where we have both a skip and an include
+        // on the same field.
+        if (directive.name.value === 'skip' || directive.name.value === 'include') {
+          if (newSelection === undefined && toBeRemoved === null) {
+            toBeRemoved = true;
+          } else if (newSelection === undefined) {
+            currSelection = selection;
+          }
+          if (newSelection) {
+            toBeRemoved = false;
+            currSelection = newSelection;
+          }
+        }
       }
     });
 
@@ -81,6 +99,8 @@ export function applyDirectivesToSelectionSet(selSet: SelectionSet,
                                                                directiveResolvers);
       }
       return newSelection;
+    } else if (!toBeRemoved) {
+      return currSelection;
     }
   });
 
@@ -89,19 +109,20 @@ export function applyDirectivesToSelectionSet(selSet: SelectionSet,
   return selSet;
 }
 
-export function skipDirectiveResolver(selection: Selection,
-                                      variables: { [name: string]: any },
-                                      directive: Directive)
+export function skipIncludeDirectiveResolver(directiveName: string,
+                                             selection: Selection,
+                                             variables: { [name: string]: any },
+                                             directive: Directive)
 : Selection {
   //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
   const directiveArguments = directive.arguments;
   if (directiveArguments.length !== 1) {
-    throw new Error('Incorrect number of arguments for the @skip directive.');
+    throw new Error(`Incorrect number of arguments for the @$(directiveName} directive.`);
   }
 
   const ifArgument = directive.arguments[0];
   if (!ifArgument.name || ifArgument.name.value !== 'if') {
-    throw new Error('Invalid argument for the @skip directive.');
+    throw new Error(`Invalid argument for the @${directiveName} directive.`);
   }
 
   const ifValue = directive.arguments[0].value;
@@ -109,25 +130,43 @@ export function skipDirectiveResolver(selection: Selection,
   if (!ifValue || ifValue.kind !== 'BooleanValue') {
     // means it has to be a variable value if this is a valid @skip directive
     if (ifValue.kind !== 'Variable') {
-      throw new Error('Invalid argument value for the @skip directive.');
+      throw new Error(`Invalid argument value for the @${directiveName} directive.`);
     } else {
       evaledValue = variables[(ifValue as Variable).name.value];
       if (evaledValue === undefined) {
-        throw new Error('Invalid variable %s referenced in @skip directive.');
+        throw new Error(`Invalid variable referenced in @${directiveName} directive.`);
       }
     }
   } else {
     evaledValue = (ifValue as BooleanValue).value;
   }
 
+  if (directiveName === 'skip') {
+    evaledValue = !evaledValue;
+  }
+
   // if the value is false, then don't skip it.
-  if (!evaledValue) {
+  if (evaledValue) {
     return selection;
   } else {
     return undefined;
   }
 }
 
+export function skipDirectiveResolver(selection: Selection,
+                                      variables: { [name: string]: any },
+                                      directive: Directive)
+: Selection {
+  return skipIncludeDirectiveResolver('skip', selection, variables, directive);
+}
+
+export function includeDirectiveResolver(selection: Selection,
+                                         variables: { [name: string]: any },
+                                         directive: Directive)
+: Selection {
+  return skipIncludeDirectiveResolver('include', selection, variables, directive);
+}
+
 export function applySkipResolver(doc: Document, variables?: { [name: string]: any })
 : Document {
   return applyDirectives(doc, variables, {
diff --git a/test/directives.ts b/test/directives.ts
index ae5215e5cde..492cfee6c31 100644
--- a/test/directives.ts
+++ b/test/directives.ts
@@ -3,6 +3,9 @@ const { assert } = chai;
 
 import {
   applySkipResolver,
+  applyDirectives,
+  skipDirectiveResolver,
+  includeDirectiveResolver,
 } from '../src/queries/directives';
 
 import gql from '../src/gql';
@@ -142,4 +145,156 @@ describe('query directives', () => {
       applySkipResolver(query, variables);
     });
   });
+
+  it('should include a field if the include is true', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @include(if: $shouldInclude)
+        }
+      }`;
+    const variables = {
+      shouldInclude: true,
+    };
+    const expQuery = gql`
+      query {
+        author {
+         firstName
+         lastName @include(if: $shouldInclude)
+        }
+     }`;
+    const newQuery = applyDirectives(query, variables, {
+      'skip': skipDirectiveResolver,
+      'include': includeDirectiveResolver,
+    });
+
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should not remove the field when skip and include are both true', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
+        }
+      }`;
+    const variables = {
+      shouldSkip: true,
+    };
+    const expQuery = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
+        }
+      }`;
+    const newQuery = applyDirectives(query, variables, {
+      'skip': skipDirectiveResolver,
+      'include': includeDirectiveResolver,
+    });
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should not remove the field when skip and include are both false', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
+        }
+      }`;
+    const variables = {
+      shouldSkip: false,
+    };
+    const expQuery = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
+        }
+      }`;
+    const newQuery = applyDirectives(query, variables, {
+      'skip': skipDirectiveResolver,
+      'include': includeDirectiveResolver,
+    });
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should remove the field when include and skip both tell it to', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @skip(if: $shouldSkip) @include(if: $shouldInclude)
+        }
+      }`;
+    const variables = {
+      shouldSkip: true,
+      shouldInclude: false,
+    };
+    const expQuery = gql`
+      query {
+        author {
+          firstName
+        }
+      }`;
+    const newQuery = applyDirectives(query, variables, {
+      'skip': skipDirectiveResolver,
+      'include': includeDirectiveResolver,
+    });
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
+  it('should not be affected by the order in which @skip and @include are presented', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
+        }
+      }`;
+    const variables1 = {
+      shouldSkip: true,
+      shouldInclude: false,
+    };
+    const variables2 = {
+      shouldSkip: false,
+      shouldInclude: false,
+    };
+    const variables3 = {
+      shouldSkip: true,
+      shouldInclude: true,
+    };
+    const expQuery1 = gql`
+      query {
+        author {
+          firstName
+        }
+      }`;
+    const expQuery2 = gql`
+      query {
+        author {
+          firstName
+          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
+        }
+      }`;
+    const expQuery3 = gql`
+      query {
+        author {
+          firstName
+          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
+        }
+      }`;
+    const newQueries = [variables1, variables2, variables3].map((variables) => {
+      return applyDirectives(query, variables, {
+        'skip': skipDirectiveResolver,
+        'include': includeDirectiveResolver,
+      });
+    });
+    assert.equal(print(newQueries[0]), print(expQuery1));
+    assert.equal(print(newQueries[1]), print(expQuery2));
+    assert.equal(print(newQueries[2]), print(expQuery3));
+  });
 });

From f53939fe6e0f68a9baeac71f9c86a060eeab9ace Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Fri, 10 Jun 2016 15:37:10 -0700
Subject: [PATCH 03/17] added more tests for include

---
 src/queries/directives.ts |  9 ++++++-
 test/directives.ts        | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 295b218f6d2..bff16f3ae2c 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -74,7 +74,7 @@ export function applyDirectivesToSelectionSet(selSet: SelectionSet,
         newSelection = directiveResolver(currSelection, variables, directive);
 
         // add handling for the case where we have both a skip and an include
-        // on the same field.
+        // on the same field (see note here: http://facebook.github.io/graphql/#sec--include).
         if (directive.name.value === 'skip' || directive.name.value === 'include') {
           if (newSelection === undefined && toBeRemoved === null) {
             toBeRemoved = true;
@@ -173,3 +173,10 @@ export function applySkipResolver(doc: Document, variables?: { [name: string]: a
     'skip': skipDirectiveResolver,
   });
 }
+
+export function applyIncludeResolver(doc: Document, variables?: { [name: string]: any })
+: Document {
+  return applyDirectives(doc, variables, {
+    'include': includeDirectiveResolver,
+  });
+}
diff --git a/test/directives.ts b/test/directives.ts
index 492cfee6c31..9c596858f92 100644
--- a/test/directives.ts
+++ b/test/directives.ts
@@ -3,6 +3,7 @@ const { assert } = chai;
 
 import {
   applySkipResolver,
+  applyIncludeResolver,
   applyDirectives,
   skipDirectiveResolver,
   includeDirectiveResolver,
@@ -172,6 +173,31 @@ describe('query directives', () => {
     assert.equal(print(newQuery), print(expQuery));
   });
 
+  it('should not include a field if the include is false', () => {
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName @include(if: $shouldInclude)
+        }
+      }`;
+    const variables = {
+      shouldInclude: false,
+    };
+    const expQuery = gql`
+      query {
+        author {
+          firstName
+        }
+      }`;
+    const newQuery = applyDirectives(query, variables, {
+      'skip': skipDirectiveResolver,
+      'include': includeDirectiveResolver,
+    });
+
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
   it('should not remove the field when skip and include are both true', () => {
     const query = gql`
       query {
@@ -222,6 +248,31 @@ describe('query directives', () => {
     assert.equal(print(newQuery), print(expQuery));
   });
 
+  it('should correctly apply include inside inline fragments', () => {
+    const query = gql`
+      query {
+        ... on RootQuery {
+          author {
+            firstName
+            lastName @include(if: $shouldInclude)
+          }
+        }
+      }`;
+    const variables = {
+      shouldInclude: false,
+    };
+    const expQuery = gql`
+      query {
+        ... on RootQuery {
+          author {
+            firstName
+          }
+        }
+      }`;
+    const newQuery = applyIncludeResolver(query, variables);
+    assert.equal(print(newQuery), print(expQuery));
+  });
+
   it('should remove the field when include and skip both tell it to', () => {
     const query = gql`
       query {

From 5588db12447b796878dad507fef6e21868ffecf5 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 10:37:38 -0700
Subject: [PATCH 04/17] in progress commit for implementing an error on extra
 fields

---
 src/data/diffAgainstStore.ts |  16 +-
 src/data/writeToStore.ts     |  29 +--
 src/queries/directives.ts    | 161 +++--------------
 test/client.ts               |  69 +++++++
 test/directives.ts           | 340 +++++------------------------------
 5 files changed, 169 insertions(+), 446 deletions(-)

diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index 21c498a1754..cda61ce3536 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -32,6 +32,10 @@ import {
   FragmentMap,
 } from '../queries/getFromAST';
 
+import {
+  shouldInclude,
+} from '../queries/directives';
+
 export interface DiffResult {
   result: any;
   isMissing?: 'true';
@@ -98,6 +102,7 @@ export function diffSelectionSetAgainstStore({
   throwOnMissingField = false,
   variables,
   fragmentMap,
+  thrownOnExtraField = true,
 }: {
   selectionSet: SelectionSet,
   store: NormalizedCache,
@@ -105,6 +110,7 @@ export function diffSelectionSetAgainstStore({
   throwOnMissingField: boolean,
   variables: Object,
   fragmentMap?: FragmentMap,
+  thrownOnExtraField?: boolean,
 }): DiffResult {
   if (selectionSet.kind !== 'SelectionSet') {
     throw new Error('Must be a selection set.');
@@ -120,6 +126,7 @@ export function diffSelectionSetAgainstStore({
   selectionSet.selections.forEach((selection) => {
     // Don't push more than one missing field per field in the query
     let missingFieldPushed = false;
+
     function pushMissingField(missingField: Selection) {
       if (!missingFieldPushed) {
         missingFields.push(missingField);
@@ -147,6 +154,7 @@ export function diffSelectionSetAgainstStore({
 
         result[resultFieldKey] = fieldResult;
       }
+
     } else if (isInlineFragment(selection)) {
       const {
         result: fieldResult,
@@ -241,8 +249,14 @@ function diffFieldAgainstStore({
 }): FieldDiffResult {
   const storeObj = store[rootId] || {};
   const storeFieldKey = storeKeyNameFromField(field, variables);
+  const included = shouldInclude(field, variables);
+
+  // check if the field is in the store when it shouldn't be
+  //if(has(storeObj, storeFieldKey) && !included) {
+  //  throw new Error(`Found extra field ${storeFieldKey} on object ${storeObj}.`);
+  //}
 
-  if (! has(storeObj, storeFieldKey)) {
+  if (! has(storeObj, storeFieldKey) && included) {
     if (throwOnMissingField) {
       throw new Error(`Can't find field ${storeFieldKey} on object ${storeObj}.`);
     }
diff --git a/src/data/writeToStore.ts b/src/data/writeToStore.ts
index 221859813bf..49a6a292e62 100644
--- a/src/data/writeToStore.ts
+++ b/src/data/writeToStore.ts
@@ -33,6 +33,10 @@ import {
   IdGetter,
 } from './extensions';
 
+import {
+  shouldInclude
+} from '../queries/directives';
+
 // import {
 //   printAST,
 // } from './debug';
@@ -115,6 +119,7 @@ export function writeSelectionSetToStore({
   variables,
   dataIdFromObject,
   fragmentMap,
+  thrownOnExtraField = true,
 }: {
   dataId: string,
   result: any,
@@ -123,6 +128,7 @@ export function writeSelectionSetToStore({
   variables: Object,
   dataIdFromObject: IdGetter,
   fragmentMap?: FragmentMap,
+  thrownOnExtraField?: boolean,
 }): NormalizedCache {
 
   if (!fragmentMap) {
@@ -134,20 +140,23 @@ export function writeSelectionSetToStore({
     if (isField(selection)) {
       const resultFieldKey: string = resultKeyNameFromField(selection);
       const value: any = result[resultFieldKey];
+      const included = shouldInclude(selection, variables);
 
-      if (isUndefined(value)) {
+      if (isUndefined(value) && included) {
         throw new Error(`Can't find field ${resultFieldKey} on result object ${dataId}.`);
       }
 
-      writeFieldToStore({
-        dataId,
-        value,
-        variables,
-        store,
-        field: selection,
-        dataIdFromObject,
-        fragmentMap,
-      });
+      if (included) {
+        writeFieldToStore({
+          dataId,
+          value,
+          variables,
+          store,
+          field: selection,
+          dataIdFromObject,
+          fragmentMap,
+        });
+      }
     } else if (isInlineFragment(selection)) {
       // XXX what to do if this tries to write the same fields? Also, type conditions...
       writeSelectionSetToStore({
diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index bff16f3ae2c..d21537d6650 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -1,125 +1,20 @@
 // Provides the methods that allow QueryManager to handle
 // the `skip` and `include` directives within GraphQL.
-
 import {
-  SelectionSet,
-  Directive,
   Selection,
-  Document,
-  InlineFragment,
-  Field,
-  BooleanValue,
   Variable,
+  BooleanValue,
+  Directive,
 } from 'graphql';
 
-import {
-  getQueryDefinition,
-  getFragmentDefinitions,
-} from './getFromAST';
-
-import identity = require('lodash.identity');
-import cloneDeep = require('lodash.clonedeep');
-
-// A handler that takes a selection, variables, and a directive to apply to that selection.
-export type DirectiveResolver = (selection: Selection,
-                                 variables: { [name: string]: any },
-                                 directive: Directive) => Selection
-
-export function applyDirectives(doc: Document,
-                                variables?: { [name: string]: any },
-                                directiveResolvers?: { [name: string]: DirectiveResolver} )
-: Document {
-  if (!variables) {
-    variables = {};
-  }
-  if (!directiveResolvers) {
-    directiveResolvers = {};
-  }
-
-  const newDoc = cloneDeep(doc);
-  const fragmentDefs = getFragmentDefinitions(newDoc);
-  const queryDef = getQueryDefinition(newDoc);
-  const newSelSet = applyDirectivesToSelectionSet(queryDef.selectionSet,
-                                                  variables,
-                                                  directiveResolvers);
-  queryDef.selectionSet = newSelSet;
-  newDoc.definitions = fragmentDefs.map((fragmentDef) => {
-    const fragmentSelSet = applyDirectivesToSelectionSet(fragmentDef.selectionSet,
-                                                         variables,
-                                                         directiveResolvers);
-    fragmentDef.selectionSet = fragmentSelSet;
-    return fragmentDef;
-  });
-  newDoc.definitions.unshift(queryDef);
-
-  return newDoc;
-}
-
-export function applyDirectivesToSelectionSet(selSet: SelectionSet,
-                                              variables: { [name: string]: any },
-                                              directiveResolvers: {
-                                                [name: string]: DirectiveResolver})
-: SelectionSet {
-
-  const selections = selSet.selections;
-  selSet.selections = selections.map((selection) => {
-
-    let newSelection: Selection = selection;
-    let currSelection: Selection = selection;
-    let toBeRemoved = null;
-
-    selection.directives.forEach((directive) => {
-      if (directive.name && directive.name.value) {
-        const directiveResolver = directiveResolvers[directive.name.value];
-        newSelection = directiveResolver(currSelection, variables, directive);
-
-        // add handling for the case where we have both a skip and an include
-        // on the same field (see note here: http://facebook.github.io/graphql/#sec--include).
-        if (directive.name.value === 'skip' || directive.name.value === 'include') {
-          if (newSelection === undefined && toBeRemoved === null) {
-            toBeRemoved = true;
-          } else if (newSelection === undefined) {
-            currSelection = selection;
-          }
-          if (newSelection) {
-            toBeRemoved = false;
-            currSelection = newSelection;
-          }
-        }
-      }
-    });
-
-    if (newSelection !== undefined) {
-      const withSelSet = selection as (InlineFragment | Field);
-      // recursively operate on selection sets within this selection set.
-      if (withSelSet.kind === 'InlineFragment' ||
-          (withSelSet.kind === 'Field' && withSelSet.selectionSet)) {
-        withSelSet.selectionSet = applyDirectivesToSelectionSet(withSelSet.selectionSet,
-                                                               variables,
-                                                               directiveResolvers);
-      }
-      return newSelection;
-    } else if (!toBeRemoved) {
-      return currSelection;
-    }
-  });
-
-  //filter out undefined values
-  selSet.selections = selSet.selections.filter(identity);
-  return selSet;
-}
-
-export function skipIncludeDirectiveResolver(directiveName: string,
-                                             selection: Selection,
-                                             variables: { [name: string]: any },
-                                             directive: Directive)
-: Selection {
+export function evaluateSkipInclude(directive: Directive, variables: { [name: string]: any }): Boolean {
   //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
   const directiveArguments = directive.arguments;
   if (directiveArguments.length !== 1) {
     throw new Error(`Incorrect number of arguments for the @$(directiveName} directive.`);
   }
 
+  const directiveName = directive.name.value;
   const ifArgument = directive.arguments[0];
   if (!ifArgument.name || ifArgument.name.value !== 'if') {
     throw new Error(`Invalid argument for the @${directiveName} directive.`);
@@ -128,7 +23,7 @@ export function skipIncludeDirectiveResolver(directiveName: string,
   const ifValue = directive.arguments[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 directive
+    // means it has to be a variable value if this is a valid @skip or @include directive
     if (ifValue.kind !== 'Variable') {
       throw new Error(`Invalid argument value for the @${directiveName} directive.`);
     } else {
@@ -145,38 +40,26 @@ export function skipIncludeDirectiveResolver(directiveName: string,
     evaledValue = !evaledValue;
   }
 
-  // if the value is false, then don't skip it.
-  if (evaledValue) {
-    return selection;
-  } else {
-    return undefined;
-  }
+  return evaledValue;
 }
 
-export function skipDirectiveResolver(selection: Selection,
-                                      variables: { [name: string]: any },
-                                      directive: Directive)
-: Selection {
-  return skipIncludeDirectiveResolver('skip', selection, variables, directive);
-}
-
-export function includeDirectiveResolver(selection: Selection,
-                                         variables: { [name: string]: any },
-                                         directive: Directive)
-: Selection {
-  return skipIncludeDirectiveResolver('include', selection, variables, directive);
-}
+export function shouldInclude(selection: Selection, variables?: { [name: string]: any }): Boolean {
+  if (!variables) {
+    variables = {};
+  }
 
-export function applySkipResolver(doc: Document, variables?: { [name: string]: any })
-: Document {
-  return applyDirectives(doc, variables, {
-    'skip': skipDirectiveResolver,
-  });
-}
+  if (!selection.directives) {
+    return true;
+  }
 
-export function applyIncludeResolver(doc: Document, variables?: { [name: string]: any })
-: Document {
-  return applyDirectives(doc, variables, {
-    'include': includeDirectiveResolver,
+  let res: Boolean = true;
+  selection.directives.map((directive) => {
+    if (directive.name.value !== 'skip' && directive.name.value !== 'include') {
+      throw new Error(`Directive ${directive.name.value} not supported.`);
+    }
+    if (!evaluateSkipInclude(directive, variables)) {
+      res = false;
+    }
   });
+  return res;
 }
diff --git a/test/client.ts b/test/client.ts
index 89489199566..9b8bd2eacb7 100644
--- a/test/client.ts
+++ b/test/client.ts
@@ -707,6 +707,75 @@ describe('client', () => {
     });
   });
 
+  describe('directives', () => {
+    it('should be able to send a query with a skip directive true', (done) => {
+      const query = gql`
+        query {
+          fortuneCookie @skip(if: true)
+        }`;
+      const result = {};
+      const networkInterface = mockNetworkInterface(
+        {
+          request: { query },
+          result: { data: result },
+        }
+      );
+      const client = new ApolloClient({
+        networkInterface,
+      });
+      client.query({ query }).then((actualResult) => {
+        assert.deepEqual(actualResult.data, result);
+        done();
+      });
+    });
+
+    it('should be able to send a query with a skip directive false', (done) => {
+      const query = gql`
+        query {
+          fortuneCookie @skip(if: false)
+        }`;
+      const result = {
+        fortuneCookie: "result",
+      };
+      const networkInterface = mockNetworkInterface(
+        {
+          request: { query },
+          result: { data: result },
+        }
+      );
+      const client = new ApolloClient({
+        networkInterface,
+      });
+      client.query({ query }).then((actualResult) => {
+        assert.deepEqual(actualResult.data, result);
+        done();
+      });
+    });
+
+    it('should reject the query promise if skipped data arrives in the result', (done) => {
+      const query = gql`
+        query {
+          fortuneCookie @skip(if: true)
+        }`;
+      const result = {
+        fortuneCookie: "result",
+      };
+      const networkInterface = mockNetworkInterface(
+        {
+          request: { query },
+          result: { data: result },
+        }
+      );
+      const client = new ApolloClient({
+        networkInterface,
+      });
+      client.query({ query }).catch((error) => {
+        assert(error);
+        done();
+      });
+    });
+  });
+
   describe('accepts dataIdFromObject option', () => {
     const query = gql`
       query people {
diff --git a/test/directives.ts b/test/directives.ts
index 9c596858f92..2e97e11a75c 100644
--- a/test/directives.ts
+++ b/test/directives.ts
@@ -2,350 +2,98 @@ import * as chai from 'chai';
 const { assert } = chai;
 
 import {
-  applySkipResolver,
-  applyIncludeResolver,
-  applyDirectives,
-  skipDirectiveResolver,
-  includeDirectiveResolver,
+  shouldInclude,
 } from '../src/queries/directives';
 
+import {
+  getQueryDefinition,
+} from '../src/queries/getFromAST';
+
 import gql from '../src/gql';
 
-import { print } from 'graphql/language/printer';
+import cloneDeep = require('lodash.clonedeep');
 
 describe('query directives', () => {
-  it('should trim skipped fields from a selection set', () => {
+  it('should should not include a skipped field', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @skip(if: true)
-        }
-      }`;
-    const expQuery = gql`
-      query {
-        author {
-          firstName
-        }
+        fortuneCookie @skip(if: true)
       }`;
-
-    const newQuery = applySkipResolver(query);
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, {}));
   });
 
-  it('should trimmed skipped fields in queries with inlined fragments', () => {
+  it('should include an included field', () => {
     const query = gql`
       query {
-        author {
-          ...authorDetails on Author {
-            firstName
-            lastName @skip(if: true)
-          }
-        }
-      }`;
-    const expQuery = gql`
-      query {
-        author {
-          ...authorDetails on Author {
-            firstName
-          }
-        }
+        fortuneCookie @include(if: true)
       }`;
-    const newQuery = applySkipResolver(query);
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(shouldInclude(field, {}));
   });
 
-  it('should throw an error if the argument to skip is not present', () => {
+  it('should not include a not include: false field', () => {
     const query = gql`
       query {
-        author {
-          ...authorDetails on Author {
-            firstName
-            lastName @skip(someotherag: true)
-          }
-        }
-     }`;
-    assert.throws(() => {
-      applySkipResolver(query);
-    });
-  });
-
-  it('should throw an error if there are extra arguments to skip', () => {
-    const query = gql`
-      query {
-        author {
-          ... on Author {
-            firstName
-            lastName @skip(if: true, useless: false)
-          }
-        }
+        fortuneCookie @include(if: false)
       }`;
-    assert.throws(() => {
-      applySkipResolver(query);
-    });
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, {}));
   });
 
-  it('should skip stuff inside fragment spreads', () => {
+  it('should include a skip: false field', () => {
     const query = gql`
       query {
-        author {
-          ...authorDetails
-        }
-      }
-      fragment authorDetails on Author {
-        firstName
-        lastName @skip(if: true)
-      }`;
-
-    const expQuery = gql`
-      query {
-        author {
-          ...authorDetails
-        }
-      }
-      fragment authorDetails on Author {
-         firstName
+        fortuneCookie @skip(if: false)
       }`;
-
-    const newQuery = applySkipResolver(query);
-    assert.equal(print(newQuery), print(expQuery));
-  });
-
-  it('should evaluate variables for skips', () => {
-    const query = gql`
-      query myQuery($shouldSkip: Boolean) {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip)
-        }
-      }`;
-    const variables = {
-      shouldSkip: true,
-    };
-    const expQuery = gql`
-      query myQuery($shouldSkip: Boolean) {
-        author {
-          firstName
-        }
-      }`;
-
-    const newQuery = applySkipResolver(query, variables);
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(shouldInclude(field, {}));
   });
 
-  it('should throw an error if an invalid variable is referenced in the skip directive', () => {
+  it('should not include a field if skip: true and include: true', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip)
-        }
+        fortuneCookie @skip(if: true) @include(if: true)
       }`;
-    const variables = {};
-    assert.throws(() => {
-      applySkipResolver(query, variables);
-    });
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, {}));
   });
 
-  it('should include a field if the include is true', () => {
+  it('should not include a field if skip: true and include: false', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @include(if: $shouldInclude)
-        }
+        fortuneCookie @skip(if: true) @include(if: false)
       }`;
-    const variables = {
-      shouldInclude: true,
-    };
-    const expQuery = gql`
-      query {
-        author {
-         firstName
-         lastName @include(if: $shouldInclude)
-        }
-     }`;
-    const newQuery = applyDirectives(query, variables, {
-      'skip': skipDirectiveResolver,
-      'include': includeDirectiveResolver,
-    });
-
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, {}));
   });
 
-  it('should not include a field if the include is false', () => {
+  it('should include a field if skip: false and include: true', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @include(if: $shouldInclude)
-        }
-      }`;
-    const variables = {
-      shouldInclude: false,
-    };
-    const expQuery = gql`
-      query {
-        author {
-          firstName
-        }
+        fortuneCookie @skip(if:false) @include(if: true)
       }`;
-    const newQuery = applyDirectives(query, variables, {
-      'skip': skipDirectiveResolver,
-      'include': includeDirectiveResolver,
-    });
-
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(shouldInclude(field, {}));
   });
 
-  it('should not remove the field when skip and include are both true', () => {
+  it('should not include a field if skip: false and include: false', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
-        }
-      }`;
-    const variables = {
-      shouldSkip: true,
-    };
-    const expQuery = gql`
-      query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
-        }
+        fortuneCookie @skip(if: false) @include(if: false)
       }`;
-    const newQuery = applyDirectives(query, variables, {
-      'skip': skipDirectiveResolver,
-      'include': includeDirectiveResolver,
-    });
-    assert.equal(print(newQuery), print(expQuery));
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, {}));
   });
 
-  it('should not remove the field when skip and include are both false', () => {
+  it('should leave the original query unmodified', () => {
     const query = gql`
       query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
-        }
-      }`;
-    const variables = {
-      shouldSkip: false,
-    };
-    const expQuery = gql`
-      query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip) @include(if: $shouldSkip)
-        }
-      }`;
-    const newQuery = applyDirectives(query, variables, {
-      'skip': skipDirectiveResolver,
-      'include': includeDirectiveResolver,
-    });
-    assert.equal(print(newQuery), print(expQuery));
-  });
-
-  it('should correctly apply include inside inline fragments', () => {
-    const query = gql`
-      query {
-        ... on RootQuery {
-          author {
-            firstName
-            lastName @include(if: $shouldInclude)
-          }
-        }
-      }`;
-    const variables = {
-      shouldInclude: false,
-    };
-    const expQuery = gql`
-      query {
-        ... on RootQuery {
-          author {
-            firstName
-          }
-        }
-      }`;
-    const newQuery = applyIncludeResolver(query, variables);
-    assert.equal(print(newQuery), print(expQuery));
-  });
-
-  it('should remove the field when include and skip both tell it to', () => {
-    const query = gql`
-      query {
-        author {
-          firstName
-          lastName @skip(if: $shouldSkip) @include(if: $shouldInclude)
-        }
-      }`;
-    const variables = {
-      shouldSkip: true,
-      shouldInclude: false,
-    };
-    const expQuery = gql`
-      query {
-        author {
-          firstName
-        }
-      }`;
-    const newQuery = applyDirectives(query, variables, {
-      'skip': skipDirectiveResolver,
-      'include': includeDirectiveResolver,
-    });
-    assert.equal(print(newQuery), print(expQuery));
-  });
-
-  it('should not be affected by the order in which @skip and @include are presented', () => {
-    const query = gql`
-      query {
-        author {
-          firstName
-          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
-        }
-      }`;
-    const variables1 = {
-      shouldSkip: true,
-      shouldInclude: false,
-    };
-    const variables2 = {
-      shouldSkip: false,
-      shouldInclude: false,
-    };
-    const variables3 = {
-      shouldSkip: true,
-      shouldInclude: true,
-    };
-    const expQuery1 = gql`
-      query {
-        author {
-          firstName
-        }
-      }`;
-    const expQuery2 = gql`
-      query {
-        author {
-          firstName
-          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
-        }
-      }`;
-    const expQuery3 = gql`
-      query {
-        author {
-          firstName
-          lastName @include(if: $shouldInclude) @skip(if: $shouldSkip)
-        }
+        fortuneCookie @skip(if: false) @include(if: false)
       }`;
-    const newQueries = [variables1, variables2, variables3].map((variables) => {
-      return applyDirectives(query, variables, {
-        'skip': skipDirectiveResolver,
-        'include': includeDirectiveResolver,
-      });
-    });
-    assert.equal(print(newQueries[0]), print(expQuery1));
-    assert.equal(print(newQueries[1]), print(expQuery2));
-    assert.equal(print(newQueries[2]), print(expQuery3));
+    const queryClone = cloneDeep(query);
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    shouldInclude(field, {});
+    assert.deepEqual(query, queryClone);
   });
 });

From 46dab91148aeaa56c1046b84dba941876fa4dd68 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 12:34:45 -0700
Subject: [PATCH 05/17] now throwing errors if the server returns a field that
 has been skipped

---
 src/QueryManager.ts          |  2 ++
 src/data/diffAgainstStore.ts | 49 ++++++++++++++++--------------------
 src/data/writeToStore.ts     | 11 +++++---
 test/client.ts               |  7 ++++--
 4 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/src/QueryManager.ts b/src/QueryManager.ts
index 738e97dfbe0..b8aa8a98a67 100644
--- a/src/QueryManager.ts
+++ b/src/QueryManager.ts
@@ -401,6 +401,8 @@ export class QueryManager {
           let resultFromStore;
           try {
             // ensure result is combined with data already in store
+            // this will throw an error if there are missing fields that were
+            // included (e.g. @skip was false).
             resultFromStore = readSelectionSetFromStore({
               store: this.getApolloState().data,
               rootId: querySS.id,
diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index cda61ce3536..29a3a2a2926 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -102,7 +102,6 @@ export function diffSelectionSetAgainstStore({
   throwOnMissingField = false,
   variables,
   fragmentMap,
-  thrownOnExtraField = true,
 }: {
   selectionSet: SelectionSet,
   store: NormalizedCache,
@@ -110,7 +109,6 @@ export function diffSelectionSetAgainstStore({
   throwOnMissingField: boolean,
   variables: Object,
   fragmentMap?: FragmentMap,
-  thrownOnExtraField?: boolean,
 }): DiffResult {
   if (selectionSet.kind !== 'SelectionSet') {
     throw new Error('Must be a selection set.');
@@ -126,6 +124,7 @@ export function diffSelectionSetAgainstStore({
   selectionSet.selections.forEach((selection) => {
     // Don't push more than one missing field per field in the query
     let missingFieldPushed = false;
+    const included = shouldInclude(selection, variables);
 
     function pushMissingField(missingField: Selection) {
       if (!missingFieldPushed) {
@@ -135,24 +134,26 @@ export function diffSelectionSetAgainstStore({
     }
 
     if (isField(selection)) {
-      const {
-        result: fieldResult,
-        isMissing: fieldIsMissing,
-      } = diffFieldAgainstStore({
-        field: selection,
-        throwOnMissingField,
-        variables,
-        rootId,
-        store,
-        fragmentMap,
-      });
-
-      if (fieldIsMissing) {
-        pushMissingField(selection);
-      } else {
-        const resultFieldKey = resultKeyNameFromField(selection);
-
-        result[resultFieldKey] = fieldResult;
+      if (included) {
+        const {
+          result: fieldResult,
+          isMissing: fieldIsMissing,
+        } = diffFieldAgainstStore({
+          field: selection,
+          throwOnMissingField,
+          variables,
+          rootId,
+          store,
+          fragmentMap,
+        });
+
+        if (fieldIsMissing) {
+          pushMissingField(selection);
+        } else {
+          const resultFieldKey = resultKeyNameFromField(selection);
+
+          result[resultFieldKey] = fieldResult;
+        }
       }
 
     } else if (isInlineFragment(selection)) {
@@ -249,14 +250,8 @@ function diffFieldAgainstStore({
 }): FieldDiffResult {
   const storeObj = store[rootId] || {};
   const storeFieldKey = storeKeyNameFromField(field, variables);
-  const included = shouldInclude(field, variables);
-
-  // check if the field is in the store when it shouldn't be
-  //if(has(storeObj, storeFieldKey) && !included) {
-  //  throw new Error(`Found extra field ${storeFieldKey} on object ${storeObj}.`);
-  //}
 
-  if (! has(storeObj, storeFieldKey) && included) {
+  if (! has(storeObj, storeFieldKey)) {
     if (throwOnMissingField) {
       throw new Error(`Can't find field ${storeFieldKey} on object ${storeObj}.`);
     }
diff --git a/src/data/writeToStore.ts b/src/data/writeToStore.ts
index 49a6a292e62..afdb7da3b66 100644
--- a/src/data/writeToStore.ts
+++ b/src/data/writeToStore.ts
@@ -34,7 +34,7 @@ import {
 } from './extensions';
 
 import {
-  shouldInclude
+  shouldInclude,
 } from '../queries/directives';
 
 // import {
@@ -119,7 +119,6 @@ export function writeSelectionSetToStore({
   variables,
   dataIdFromObject,
   fragmentMap,
-  thrownOnExtraField = true,
 }: {
   dataId: string,
   result: any,
@@ -128,7 +127,6 @@ export function writeSelectionSetToStore({
   variables: Object,
   dataIdFromObject: IdGetter,
   fragmentMap?: FragmentMap,
-  thrownOnExtraField?: boolean,
 }): NormalizedCache {
 
   if (!fragmentMap) {
@@ -136,6 +134,7 @@ export function writeSelectionSetToStore({
     //to us for the fragments.
     fragmentMap = {};
   }
+
   selectionSet.selections.forEach((selection) => {
     if (isField(selection)) {
       const resultFieldKey: string = resultKeyNameFromField(selection);
@@ -146,7 +145,11 @@ export function writeSelectionSetToStore({
         throw new Error(`Can't find field ${resultFieldKey} on result object ${dataId}.`);
       }
 
-      if (included) {
+      if (!isUndefined(value) && !included) {
+        throw new Error(`Found extra field ${resultFieldKey} on result object ${dataId}.`);
+      }
+
+      if (!isUndefined(value)) {
         writeFieldToStore({
           dataId,
           value,
diff --git a/test/client.ts b/test/client.ts
index 9b8bd2eacb7..b01cdc0e1bb 100644
--- a/test/client.ts
+++ b/test/client.ts
@@ -735,7 +735,7 @@ describe('client', () => {
           fortuneCookie @skip(if: false)
         }`;
       const result = {
-        fortuneCookie: "result",
+        fortuneCookie: 'result',
       };
       const networkInterface = mockNetworkInterface(
         {
@@ -756,9 +756,11 @@ describe('client', () => {
       const query = gql`
         query {
           fortuneCookie @skip(if: true)
+          otherThing
         }`;
       const result = {
-        fortuneCookie: "result",
+        fortuneCookie: 'you will go far',
+        otherThing: 'false',
       };
       const networkInterface = mockNetworkInterface(
         {
@@ -769,6 +771,7 @@ describe('client', () => {
       const client = new ApolloClient({
         networkInterface,
       });
+
       client.query({ query }).catch((error) => {
         assert(error);
         done();

From 7e1395a9411ae391c365547102f7532618e6d799 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 13:45:46 -0700
Subject: [PATCH 06/17] added tests that should make coveralls happier

---
 test/directives.ts | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/test/directives.ts b/test/directives.ts
index 2e97e11a75c..afbc03cc646 100644
--- a/test/directives.ts
+++ b/test/directives.ts
@@ -96,4 +96,86 @@ describe('query directives', () => {
     shouldInclude(field, {});
     assert.deepEqual(query, queryClone);
   });
+
+  it('throws an error on an unsupported directive', () => {
+    const query = gql`
+      query {
+        fortuneCookie @dosomething(if: true)
+      }`;
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+
+    assert.throws(() => {
+      shouldInclude(field, {});
+    });
+  });
+
+  it('throws an error on an invalid argument for the skip directive', () => {
+    const query = gql`
+      query {
+        fortuneCookie @skip(nothing: true)
+      }`;
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+
+    assert.throws(() => {
+      shouldInclude(field, {});
+    });
+  });
+
+  it('throws an error on an invalid argument for the include directive', () => {
+    const query = gql`
+      query {
+        fortuneCookie @include(nothing: true)
+      }`;
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+
+    assert.throws(() => {
+      shouldInclude(field, {});
+    });
+  });
+
+  it('throws an error on an invalid variable name within a directive argument', () => {
+    const query = gql`
+      query {
+        fortuneCookie @include(if: $neverDefined)
+      }`;
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert.throws(() => {
+      shouldInclude(field, {});
+    });
+  });
+
+  it('evaluates variables on skip fields', () => {
+    const query = gql`
+      query($shouldSkip: Boolean) {
+        fortuneCookie @skip(if: $shouldSkip)
+      }`;
+    const variables = {
+      shouldSkip: true,
+    };
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, variables));
+  });
+
+  it('evaluates variables on include fields', () => {
+    const query = gql`
+      query($shouldSkip: Boolean) {
+        fortuneCookie @include(if: $shouldInclude)
+      }`;
+    const variables = {
+      shouldInclude: false,
+    };
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert(!shouldInclude(field, variables));
+  });
+
+  it('throws an error if the value of the argument is not a variable or boolean', () => {
+    const query = gql`
+      query {
+        fortuneCookie @include(if: "string")
+      }`;
+    const field = getQueryDefinition(query).selectionSet.selections[0];
+    assert.throws(() => {
+      shouldInclude(field, {});
+    });
+  });
 });

From 28b9a417e191c940789ad4f3ca7cf1b9869faebd Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 14:50:41 -0700
Subject: [PATCH 07/17] fixed one of the tests so that it now doesn't print out
 a bunch of garbage when it runs

---
 src/data/diffAgainstStore.ts |  2 ++
 src/store.ts                 | 10 +++++++++-
 test/client.ts               | 23 +++++++++++++++++++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index 29a3a2a2926..0a52635a771 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -154,6 +154,8 @@ export function diffSelectionSetAgainstStore({
 
           result[resultFieldKey] = fieldResult;
         }
+      } else {
+        pushMissingField(selection);
       }
 
     } else if (isInlineFragment(selection)) {
diff --git a/src/store.ts b/src/store.ts
index 8df4b7d6e3d..3a299a84b1a 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -72,13 +72,19 @@ export function createApolloStore({
   reduxRootKey = 'apollo',
   initialState,
   config = {},
+  reportCrashes,
 }: {
   reduxRootKey?: string,
   initialState?: any,
   config?: ApolloReducerConfig,
+  reportCrashes?: boolean,
 } = {}): ApolloStore {
   const enhancers = [];
 
+  if (reportCrashes === undefined) {
+    reportCrashes = true;
+  }
+
   if (typeof window !== 'undefined') {
     const anyWindow = window as any;
     if (anyWindow.devToolsExtension) {
@@ -86,7 +92,9 @@ export function createApolloStore({
     }
   }
 
-  enhancers.push(applyMiddleware(crashReporter));
+  if (reportCrashes) {
+    enhancers.push(applyMiddleware(crashReporter));
+  }
 
   return createStore(
     combineReducers({ [reduxRootKey]: createApolloReducer(config) }),
diff --git a/test/client.ts b/test/client.ts
index b01cdc0e1bb..9a9353ce556 100644
--- a/test/client.ts
+++ b/test/client.ts
@@ -26,6 +26,14 @@ import {
   applyMiddleware,
 } from 'redux';
 
+import {
+  createApolloStore,
+} from '../src/store';
+
+import {
+  QueryManager,
+} from '../src/QueryManager';
+
 import {
   createNetworkInterface,
   HTTPNetworkInterface,
@@ -771,11 +779,22 @@ describe('client', () => {
       const client = new ApolloClient({
         networkInterface,
       });
+      // we need this so it doesn't print out a bunch of stuff we don't need
+      // when we're trying to test an exception.
+      client.store = createApolloStore({ reportCrashes: false });
+      client.queryManager = new QueryManager({
+        networkInterface,
+        store: client.store,
+        reduxRootKey: 'apollo',
+      });
 
-      client.query({ query }).catch((error) => {
-        assert(error);
+      client.query({ query }).then(() => {
+        // do nothing
+      }).catch((error) => {
+        assert.include(error.message, 'Found extra field');
         done();
       });
+
     });
   });
 

From ca7518bfdd61d6e6065f8ed0945761128fd3b5ba Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 14:57:52 -0700
Subject: [PATCH 08/17] fixed an issue where the skipped fields would not get
 sent to the server as part of a query even if they weren't in the store

---
 src/data/diffAgainstStore.ts | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index 0a52635a771..b29e37aa35e 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -134,7 +134,6 @@ export function diffSelectionSetAgainstStore({
     }
 
     if (isField(selection)) {
-      if (included) {
         const {
           result: fieldResult,
           isMissing: fieldIsMissing,
@@ -145,19 +144,19 @@ export function diffSelectionSetAgainstStore({
           rootId,
           store,
           fragmentMap,
+          included,
         });
 
-        if (fieldIsMissing) {
-          pushMissingField(selection);
-        } else {
-          const resultFieldKey = resultKeyNameFromField(selection);
-
-          result[resultFieldKey] = fieldResult;
-        }
-      } else {
+      if (fieldIsMissing) {
+        // even if the field is not included, we want to keep it in the
+        // query that is sent to the server. So, we push it into the set of
+        // fields that is missing.
         pushMissingField(selection);
-      }
+      } else if (included) {
+        const resultFieldKey = resultKeyNameFromField(selection);
 
+        result[resultFieldKey] = fieldResult;
+      }
     } else if (isInlineFragment(selection)) {
       const {
         result: fieldResult,
@@ -242,6 +241,7 @@ function diffFieldAgainstStore({
   rootId,
   store,
   fragmentMap,
+  included,
 }: {
   field: Field,
   throwOnMissingField: boolean,
@@ -249,12 +249,16 @@ function diffFieldAgainstStore({
   rootId: string,
   store: NormalizedCache,
   fragmentMap?: FragmentMap,
+  included?: Boolean,
 }): FieldDiffResult {
   const storeObj = store[rootId] || {};
   const storeFieldKey = storeKeyNameFromField(field, variables);
+  if (included === undefined) {
+    included = true;
+  }
 
   if (! has(storeObj, storeFieldKey)) {
-    if (throwOnMissingField) {
+    if (throwOnMissingField && included) {
       throw new Error(`Can't find field ${storeFieldKey} on object ${storeObj}.`);
     }
 

From 88704622e0b3fa186a4d23c130e17b464087cbf4 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 15:04:54 -0700
Subject: [PATCH 09/17] fixed issues @stubailo had mentioned

---
 src/QueryManager.ts       | 4 ++--
 src/queries/directives.ts | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/QueryManager.ts b/src/QueryManager.ts
index b8aa8a98a67..efb47d3816d 100644
--- a/src/QueryManager.ts
+++ b/src/QueryManager.ts
@@ -401,8 +401,8 @@ export class QueryManager {
           let resultFromStore;
           try {
             // ensure result is combined with data already in store
-            // this will throw an error if there are missing fields that were
-            // included (e.g. @skip was false).
+            // this will throw an error if there are missing fields in
+            // the results if returnPartialData is false.
             resultFromStore = readSelectionSetFromStore({
               store: this.getApolloState().data,
               rootId: querySS.id,
diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index d21537d6650..3a54fe0d4e7 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -7,14 +7,16 @@ import {
   Directive,
 } from 'graphql';
 
-export function evaluateSkipInclude(directive: Directive, variables: { [name: string]: any }): Boolean {
+export function evaluateSkipInclude(directive: Directive,
+  variables: { [name: string]: any }): Boolean {
   //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
   const directiveArguments = directive.arguments;
+  const directiveName = directive.name.value;
   if (directiveArguments.length !== 1) {
-    throw new Error(`Incorrect number of arguments for the @$(directiveName} directive.`);
+    throw new Error(`Incorrect number of arguments for the @${directiveName} directive.`);
   }
 
-  const directiveName = directive.name.value;
+
   const ifArgument = directive.arguments[0];
   if (!ifArgument.name || ifArgument.name.value !== 'if') {
     throw new Error(`Invalid argument for the @${directiveName} directive.`);

From 20c1ea4cd9fe3cadc7cc99a2abdd55ef869cf0bd Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Thu, 16 Jun 2016 15:08:28 -0700
Subject: [PATCH 10/17] updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5c7f4ce363c..591795129a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@ Expect active development and potentially significant breaking changes in the `0
 
 ### vNEXT
 - Added name fragment support within mutations [Issue #273](https://github.com/apollostack/apollo-client/issues/273) and [PR #274](https://github.com/apollostack/apollo-client/pull/274)
+- Added support for `@skip` and `@include` directives - see [Issue #237](https://github.com/apollostack/apollo-client/issues/237) and [PR #275](https://github.com/apollostack/apollo-client/pull/275)
 
 ### v0.3.13
 - Removed AuthTokenHeaderMiddleware code and related tests from apollo-client [Issue #247](https://github.com/apollostack/apollo-client/issues/247)

From 7cd95d571bb3c4c0878fc46bbf6be548876226cd Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Fri, 17 Jun 2016 13:23:04 -0700
Subject: [PATCH 11/17] fixed issues with shouldInclude

---
 src/data/diffAgainstStore.ts |  6 +--
 src/queries/directives.ts    | 76 ++++++++++++++++++------------------
 2 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index b29e37aa35e..239ebcd15dd 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -124,7 +124,6 @@ export function diffSelectionSetAgainstStore({
   selectionSet.selections.forEach((selection) => {
     // Don't push more than one missing field per field in the query
     let missingFieldPushed = false;
-    const included = shouldInclude(selection, variables);
 
     function pushMissingField(missingField: Selection) {
       if (!missingFieldPushed) {
@@ -134,6 +133,7 @@ export function diffSelectionSetAgainstStore({
     }
 
     if (isField(selection)) {
+        const includeField = shouldInclude(selection, variables);
         const {
           result: fieldResult,
           isMissing: fieldIsMissing,
@@ -144,7 +144,7 @@ export function diffSelectionSetAgainstStore({
           rootId,
           store,
           fragmentMap,
-          included,
+          included: includeField,
         });
 
       if (fieldIsMissing) {
@@ -152,7 +152,7 @@ export function diffSelectionSetAgainstStore({
         // query that is sent to the server. So, we push it into the set of
         // fields that is missing.
         pushMissingField(selection);
-      } else if (included) {
+      } else if (includeField) {
         const resultFieldKey = resultKeyNameFromField(selection);
 
         result[resultFieldKey] = fieldResult;
diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 3a54fe0d4e7..7dd177bacd6 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -4,46 +4,8 @@ import {
   Selection,
   Variable,
   BooleanValue,
-  Directive,
 } from 'graphql';
 
-export function evaluateSkipInclude(directive: Directive,
-  variables: { [name: string]: any }): Boolean {
-  //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
-  const directiveArguments = directive.arguments;
-  const directiveName = directive.name.value;
-  if (directiveArguments.length !== 1) {
-    throw new Error(`Incorrect number of arguments for the @${directiveName} directive.`);
-  }
-
-
-  const ifArgument = directive.arguments[0];
-  if (!ifArgument.name || ifArgument.name.value !== 'if') {
-    throw new Error(`Invalid argument for the @${directiveName} directive.`);
-  }
-
-  const ifValue = directive.arguments[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
-    if (ifValue.kind !== 'Variable') {
-      throw new Error(`Invalid argument value for the @${directiveName} directive.`);
-    } else {
-      evaledValue = variables[(ifValue as Variable).name.value];
-      if (evaledValue === undefined) {
-        throw new Error(`Invalid variable referenced in @${directiveName} directive.`);
-      }
-    }
-  } else {
-    evaledValue = (ifValue as BooleanValue).value;
-  }
-
-  if (directiveName === 'skip') {
-    evaledValue = !evaledValue;
-  }
-
-  return evaledValue;
-}
 
 export function shouldInclude(selection: Selection, variables?: { [name: string]: any }): Boolean {
   if (!variables) {
@@ -56,12 +18,48 @@ export function shouldInclude(selection: Selection, variables?: { [name: string]
 
   let res: Boolean = true;
   selection.directives.map((directive) => {
+    // TODO should move this validation to GraphQL validation once that's implemented.
     if (directive.name.value !== 'skip' && directive.name.value !== 'include') {
       throw new Error(`Directive ${directive.name.value} not supported.`);
     }
-    if (!evaluateSkipInclude(directive, variables)) {
+
+    //evaluate the "if" argument and skip (i.e. return undefined) if it evaluates to true.
+    const directiveArguments = directive.arguments;
+    const directiveName = directive.name.value;
+    if (directiveArguments.length !== 1) {
+      throw new Error(`Incorrect number of arguments for the @${directiveName} directive.`);
+    }
+
+
+    const ifArgument = directive.arguments[0];
+    if (!ifArgument.name || ifArgument.name.value !== 'if') {
+      throw new Error(`Invalid argument for the @${directiveName} directive.`);
+    }
+
+    const ifValue = directive.arguments[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
+      if (ifValue.kind !== 'Variable') {
+        throw new Error(`Invalid argument value for the @${directiveName} directive.`);
+      } else {
+        evaledValue = variables[(ifValue as Variable).name.value];
+        if (evaledValue === undefined) {
+          throw new Error(`Invalid variable referenced in @${directiveName} directive.`);
+        }
+      }
+    } else {
+      evaledValue = (ifValue as BooleanValue).value;
+    }
+
+    if (directiveName === 'skip') {
+      evaledValue = !evaledValue;
+    }
+
+    if (!evaledValue) {
       res = false;
     }
   });
+
   return res;
 }

From 0c1e74c926a2e271a550008e629c24963b30edd5 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Fri, 17 Jun 2016 13:24:56 -0700
Subject: [PATCH 12/17] fixed style issue

---
 src/queries/directives.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 7dd177bacd6..4a675107e40 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -17,7 +17,7 @@ export function shouldInclude(selection: Selection, variables?: { [name: string]
   }
 
   let res: Boolean = true;
-  selection.directives.map((directive) => {
+  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') {
       throw new Error(`Directive ${directive.name.value} not supported.`);

From bced2d4dfc17f8c76bbd7302b90431759819c497 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Tue, 21 Jun 2016 11:23:09 -0700
Subject: [PATCH 13/17] fixes

---
 src/data/diffAgainstStore.ts | 5 +----
 src/queries/directives.ts    | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
index 239ebcd15dd..673821d4396 100644
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -241,7 +241,7 @@ function diffFieldAgainstStore({
   rootId,
   store,
   fragmentMap,
-  included,
+  included = true,
 }: {
   field: Field,
   throwOnMissingField: boolean,
@@ -253,9 +253,6 @@ function diffFieldAgainstStore({
 }): FieldDiffResult {
   const storeObj = store[rootId] || {};
   const storeFieldKey = storeKeyNameFromField(field, variables);
-  if (included === undefined) {
-    included = true;
-  }
 
   if (! has(storeObj, storeFieldKey)) {
     if (throwOnMissingField && included) {
diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 4a675107e40..82974d22b0c 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -27,7 +27,7 @@ export function shouldInclude(selection: Selection, variables?: { [name: string]
     const directiveArguments = directive.arguments;
     const directiveName = directive.name.value;
     if (directiveArguments.length !== 1) {
-      throw new Error(`Incorrect number of arguments for the @${directiveName} directive.`);
+      throw new Error(`Argument for the @${directiveName} directive must be a boolean value or variable.`);
     }
 
 

From 4bda736674f310e75ba3c479f7d46a34bf2a7491 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Tue, 21 Jun 2016 11:26:50 -0700
Subject: [PATCH 14/17] directives

---
 #gql.js#                  | 5 +++++
 .#gql.js                  | 1 +
 queryTransform.js         | 1 +
 queryTransform.js~        | 1 +
 src/queries/directives.ts | 4 ++--
 5 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 #gql.js#
 create mode 120000 .#gql.js
 create mode 100644 queryTransform.js
 create mode 100644 queryTransform.js~

diff --git a/#gql.js# b/#gql.js#
new file mode 100644
index 00000000000..ea2848e0b31
--- /dev/null
+++ b/#gql.js#
@@ -0,0 +1,5 @@
+/* We are placing this file in the root to enable npm-link development
+ * Currently, gql resides in a submodule and is not able to be imported when linked
+ */
+module.exports = require('./lib/src/gql');
+x
\ No newline at end of file
diff --git a/.#gql.js b/.#gql.js
new file mode 120000
index 00000000000..951dfa5557e
--- /dev/null
+++ b/.#gql.js
@@ -0,0 +1 @@
+dhaivatpandya@Dhaivats-MacBook-Air-2.local.428
\ No newline at end of file
diff --git a/queryTransform.js b/queryTransform.js
new file mode 100644
index 00000000000..85e094b7051
--- /dev/null
+++ b/queryTransform.js
@@ -0,0 +1 @@
+module.exports = require('../lib/src/queries/queryTransform');
diff --git a/queryTransform.js~ b/queryTransform.js~
new file mode 100644
index 00000000000..6f61136b279
--- /dev/null
+++ b/queryTransform.js~
@@ -0,0 +1 @@
+module.exports = require('./src/queries/queryTransform')
diff --git a/src/queries/directives.ts b/src/queries/directives.ts
index 82974d22b0c..db24617c893 100644
--- a/src/queries/directives.ts
+++ b/src/queries/directives.ts
@@ -27,7 +27,7 @@ export function shouldInclude(selection: Selection, variables?: { [name: string]
     const directiveArguments = directive.arguments;
     const directiveName = directive.name.value;
     if (directiveArguments.length !== 1) {
-      throw new Error(`Argument for the @${directiveName} directive must be a boolean value or variable.`);
+      throw new Error(`Incorrect number of arguments for the @${directiveName} directive.`);
     }
 
 
@@ -41,7 +41,7 @@ export function shouldInclude(selection: Selection, variables?: { [name: string]
     if (!ifValue || ifValue.kind !== 'BooleanValue') {
       // means it has to be a variable value if this is a valid @skip or @include directive
       if (ifValue.kind !== 'Variable') {
-        throw new Error(`Invalid argument value for the @${directiveName} directive.`);
+        throw new Error(`Argument for the @${directiveName} directive must be a variable or a bool ean value.`);
       } else {
         evaledValue = variables[(ifValue as Variable).name.value];
         if (evaledValue === undefined) {

From a451fd21889835613c6a002c8045ba3d66a29096 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Tue, 21 Jun 2016 11:27:38 -0700
Subject: [PATCH 15/17] somehow these fixes didn't work last commit

---
 #gql.js#           | 5 -----
 .#gql.js           | 1 -
 queryTransform.js  | 1 -
 queryTransform.js~ | 1 -
 4 files changed, 8 deletions(-)
 delete mode 100644 #gql.js#
 delete mode 120000 .#gql.js
 delete mode 100644 queryTransform.js
 delete mode 100644 queryTransform.js~

diff --git a/#gql.js# b/#gql.js#
deleted file mode 100644
index ea2848e0b31..00000000000
--- a/#gql.js#
+++ /dev/null
@@ -1,5 +0,0 @@
-/* We are placing this file in the root to enable npm-link development
- * Currently, gql resides in a submodule and is not able to be imported when linked
- */
-module.exports = require('./lib/src/gql');
-x
\ No newline at end of file
diff --git a/.#gql.js b/.#gql.js
deleted file mode 120000
index 951dfa5557e..00000000000
--- a/.#gql.js
+++ /dev/null
@@ -1 +0,0 @@
-dhaivatpandya@Dhaivats-MacBook-Air-2.local.428
\ No newline at end of file
diff --git a/queryTransform.js b/queryTransform.js
deleted file mode 100644
index 85e094b7051..00000000000
--- a/queryTransform.js
+++ /dev/null
@@ -1 +0,0 @@
-module.exports = require('../lib/src/queries/queryTransform');
diff --git a/queryTransform.js~ b/queryTransform.js~
deleted file mode 100644
index 6f61136b279..00000000000
--- a/queryTransform.js~
+++ /dev/null
@@ -1 +0,0 @@
-module.exports = require('./src/queries/queryTransform')

From 5df4a875605a4b2c7f8ca319ee9c0e2557843c44 Mon Sep 17 00:00:00 2001
From: Dhaivat Pandya <dhaivat@meteor.com>
Date: Tue, 21 Jun 2016 13:47:44 -0700
Subject: [PATCH 16/17] moved tests to roundtrip.ts

---
 test/client.ts    | 44 --------------------------------------------
 test/roundtrip.ts | 18 ++++++++++++++++++
 2 files changed, 18 insertions(+), 44 deletions(-)

diff --git a/test/client.ts b/test/client.ts
index af065209b83..796b451fc41 100644
--- a/test/client.ts
+++ b/test/client.ts
@@ -719,50 +719,6 @@ describe('client', () => {
   });
 
   describe('directives', () => {
-    it('should be able to send a query with a skip directive true', (done) => {
-      const query = gql`
-        query {
-          fortuneCookie @skip(if: true)
-        }`;
-      const result = {};
-      const networkInterface = mockNetworkInterface(
-        {
-          request: { query },
-          result: { data: result },
-        }
-      );
-      const client = new ApolloClient({
-        networkInterface,
-      });
-      client.query({ query }).then((actualResult) => {
-        assert.deepEqual(actualResult.data, result);
-        done();
-      });
-    });
-
-    it('should be able to send a query with a skip directive false', (done) => {
-      const query = gql`
-        query {
-          fortuneCookie @skip(if: false)
-        }`;
-      const result = {
-        fortuneCookie: 'result',
-      };
-      const networkInterface = mockNetworkInterface(
-        {
-          request: { query },
-          result: { data: result },
-        }
-      );
-      const client = new ApolloClient({
-        networkInterface,
-      });
-      client.query({ query }).then((actualResult) => {
-        assert.deepEqual(actualResult.data, result);
-        done();
-      });
-    });
-
     it('should reject the query promise if skipped data arrives in the result', (done) => {
       const query = gql`
         query {
diff --git a/test/roundtrip.ts b/test/roundtrip.ts
index 63df448a210..fa33ff5e9af 100644
--- a/test/roundtrip.ts
+++ b/test/roundtrip.ts
@@ -91,6 +91,24 @@ describe('roundtrip', () => {
       },
     });
   });
+
+  describe('directives', () => {
+    it('should be able to query with skip directive true', () => {
+      storeRoundtrip(gql`
+        query {
+          fortuneCookie @skip(if: true)
+        }
+      `, {});
+    });
+
+    it('should be able to query with skip directive false', () => {
+      storeRoundtrip(gql`
+        query {
+          fortuneCookie @skip(if: false)
+        }
+      `, {fortuneCookie: 'live long and prosper'});
+    });
+  });
 });
 
 function storeRoundtrip(query: Document, result, variables = {}) {

From 85a7585d12cd19519e9b8a2cba91f2ef4f7a39be Mon Sep 17 00:00:00 2001
From: Sashko Stubailo <sashko@stubailo.com>
Date: Tue, 21 Jun 2016 13:57:37 -0700
Subject: [PATCH 17/17] Fix changelog

---
 CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 56cc658efd0..76b0fe2d042 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,11 +4,12 @@ Expect active development and potentially significant breaking changes in the `0
 
 ### vNEXT
 
+- Added support for `@skip` and `@include` directives - see [Issue #237](https://github.com/apollostack/apollo-client/issues/237) and [PR #275](https://github.com/apollostack/apollo-client/pull/275)
+
 ### v0.3.14
 
 - Added support for inline object and array arguments in queries and mutations, where previously you had to use variables. [PR #252](https://github.com/apollostack/apollo-client/pull/252)
 - Added name fragment support within mutations [Issue #273](https://github.com/apollostack/apollo-client/issues/273) and [PR #274](https://github.com/apollostack/apollo-client/pull/274)
-- Added support for `@skip` and `@include` directives - see [Issue #237](https://github.com/apollostack/apollo-client/issues/237) and [PR #275](https://github.com/apollostack/apollo-client/pull/275)
 - Now sending the operation name along with the query to the server [Issue #259](https://github.com/apollostack/apollo-client/issues/259) and [PR #282](https://github.com/apollostack/apollo-client/pull/282)
 
 ### v0.3.13