Skip should prevent options from being called when component properties were updated (apollographql#1181)

* Should declare undefined variables within the options property when skipping

* Check if should skip before calling the mapPropsToOptions method

* Create a separated test.

* Add the fix to the Changelog.md file

Author: rhberro

Changelog.md

@@ -1,14 +1,15 @@
 # Change log
 
 ### vNext
+- fix skip on component update.
+- Fix: ensure `client` option can be used with mutation query [#1145](https://github.com/apollographql/react-apollo/pull/1145)
 
 ### 1.4.16
 - upgrade to react-16
 - fix shallowEqual bug.
 - Added notifyOnNetworkStatusChange to QueryOpts and MutationOpts Typesccript definitions [#1034](https://github.com/apollographql/react-apollo/pull/1034)
 - Added variables types with Typescript [#997](https://github.com/apollographql/react-apollo/pull/997)
 - Made `ChildProps.data` non-optional [#1143](https://github.com/apollographql/react-apollo/pull/1143)
-- Fix: ensure `client` option can be used with mutation query [#1145](https://github.com/apollographql/react-apollo/pull/1145)
 
 ### 1.4.15
 - Fix: handle calling refetch in child componentDidMount

src/graphql.tsx

@@ -172,7 +172,16 @@ export default function graphql<
       }
 
       componentWillReceiveProps(nextProps, nextContext) {
+        if (this.shouldSkip(nextProps)) {
+          if (!this.shouldSkip(this.props)) {
+            // if this has changed, we better unsubscribe
+            this.unsubscribeFromQuery();
+          }
+          return;
+        }
+
         const { client } = mapPropsToOptions(nextProps);
+
         if (
           shallowEqual(this.props, nextProps) &&
           (this.client === client || this.client === nextContext.client)
@@ -215,14 +224,6 @@ export default function graphql<
           return;
         }
 
-        if (this.shouldSkip(nextProps)) {
-          if (!this.shouldSkip(this.props)) {
-            // if this has changed, we better unsubscribe
-            this.unsubscribeFromQuery();
-          }
-          return;
-        }
-
         this.updateQuery(nextProps);
         this.subscribeToQuery();
       }

test/react-web/client/graphql/queries/skip.test.tsx

@@ -300,6 +300,64 @@ describe('[queries] skip', () => {
     }, 25);
   });
 
+  it("doesn't run options or props when skipped even if the component updates", done => {
+    const query = gql`
+      query people {
+        allPeople(first: 1) {
+          people {
+            name
+          }
+        }
+      }
+    `;
+
+    const networkInterface = mockNetworkInterface();
+
+    const client = new ApolloClient({ networkInterface, addTypename: false });
+
+    let queryWasSkipped = true;
+
+    @graphql(query, {
+      skip: true,
+      options: () => {
+        queryWasSkipped = false;
+        return {};
+      },
+      props: () => {
+        queryWasSkipped = false;
+        return {};
+      },
+    })
+    class Container extends React.Component<any, any> {
+      componentWillReceiveProps(props) {
+        expect(queryWasSkipped).toBe(true);
+        done();
+      }
+      render() {
+        return null;
+      }
+    }
+
+    class Parent extends React.Component<any, any> {
+      constructor() {
+        super();
+        this.state = { foo: 'bar' };
+      }
+      componentDidMount() {
+        this.setState({ foo: 'baz' });
+      }
+      render() {
+        return <Container foo={this.state.foo} />;
+      }
+    }
+
+    renderer.create(
+      <ApolloProvider client={client}>
+        <Parent />
+      </ApolloProvider>,
+    );
+  });
+
   it('allows you to skip a query without running it (alternate syntax)', done => {
     const query = gql`
       query people {