Forward-port #1181 to 2.0 (#1330)

* Skip should prevent options from call in componentWillReceiveProps

Identical to #1181 which didn't make it to 2.0.

Test fails because it uses networkInterface. Next commit will fix for 2.0 API.

* Update test to apollo-client 2.0

Changelog.md

@@ -8,6 +8,7 @@
 - Support passing an updater function to `setState` in SSR mode [#1263](https://github.com/apollographql/react-apollo/pull/1263)
 - Correctly initializes component state as null (not undefined) [#1300](https://github.com/apollographql/react-apollo/pull/1310)
 - Correctly provide the generic cache type to ApolloProvider [#1319](https://github.com/apollographql/react-apollo/pull/1319)
+- fix skip on component update [#1330](https://github.com/apollographql/react-apollo/pull/1330)
 
 ### 2.0.0-beta.0
 - upgrade to Apollo Client 2.0

src/graphql.tsx

@@ -169,7 +169,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)
@@ -209,13 +218,6 @@ export default function graphql<
           this.subscribeToQuery();
           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

@@ -314,6 +314,70 @@ 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 link = mockSingleLink({
+      request: { query },
+      result: {},
+    });
+
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ 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 {