Do not update query after changing ApiClient (apollographql#1145)

* Do not update query after changing ApiClient if mutation is provided to the container

* Added test case and changelog entry for apollographql#1145

Author: valerybugakov

Changelog.md

@@ -3,11 +3,12 @@
 ### vNext
 
 ### 1.4.16
-- upgrade to react-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

@@ -163,7 +163,10 @@ export default function graphql<
           // call any stacked refetch functions
           if (this.refetcherQueue) {
             const { args, resolve, reject } = this.refetcherQueue;
-            this.queryObservable.refetch(args).then(resolve).catch(reject);
+            this.queryObservable
+              .refetch(args)
+              .then(resolve)
+              .catch(reject);
           }
         }
       }
@@ -179,6 +182,10 @@ export default function graphql<
 
         this.shouldRerender = true;
 
+        if (this.type === DocumentType.Mutation) {
+          return;
+        }
+
         if (this.client !== client && this.client !== nextContext.client) {
           if (client) {
             this.client = client;
@@ -195,9 +202,7 @@ export default function graphql<
           }
           return;
         }
-        if (this.type === DocumentType.Mutation) {
-          return;
-        }
+
         if (
           this.type === DocumentType.Subscription &&
           operationOptions.shouldResubscribe &&
@@ -209,6 +214,7 @@ export default function graphql<
           this.subscribeToQuery();
           return;
         }
+
         if (this.shouldSkip(nextProps)) {
           if (!this.shouldSkip(this.props)) {
             // if this has changed, we better unsubscribe

test/react-web/client/graphql/mutations/lifecycle.test.tsx

@@ -5,30 +5,36 @@ import assign = require('object-assign');
 
 import ApolloClient from 'apollo-client';
 
-declare function require(name: string)
+declare function require(name: string);
 
 import { mockNetworkInterface } from '../../../../../src/test-utils';
 
 import { ApolloProvider, graphql } from '../../../../../src';
 
+const query = gql`
+  mutation addPerson($id: Int) {
+    allPeople(id: $id) {
+      people {
+        name
+      }
+    }
+  }
+`;
+
+const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
+
+const createClient = variables => {
+  const networkInterface = mockNetworkInterface({
+    request: { query, variables },
+    result: { data },
+  });
+
+  return new ApolloClient({ networkInterface, addTypename: false });
+};
+
 describe('[mutations] lifecycle', () => {
   it('allows falsy values in the mapped variables from props', done => {
-    const query = gql`
-      mutation addPerson($id: Int) {
-        allPeople(id: $id) {
-          people {
-            name
-          }
-        }
-      }
-    `;
-    const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
-    const variables = { id: null };
-    const networkInterface = mockNetworkInterface({
-      request: { query, variables },
-      result: { data },
-    });
-    const client = new ApolloClient({ networkInterface, addTypename: false });
+    const client = createClient({ id: null });
 
     @graphql(query)
     class Container extends React.Component<any, any> {
@@ -51,22 +57,7 @@ describe('[mutations] lifecycle', () => {
   });
 
   it("errors if the passed props don't contain the needed variables", () => {
-    const query = gql`
-      mutation addPerson($first: Int) {
-        allPeople(first: $first) {
-          people {
-            name
-          }
-        }
-      }
-    `;
-    const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
-    const variables = { first: 1 };
-    const networkInterface = mockNetworkInterface({
-      request: { query, variables },
-      result: { data },
-    });
-    const client = new ApolloClient({ networkInterface, addTypename: false });
+    const client = createClient({ first: 1 });
     const Container = graphql(query)(() => null);
 
     try {
@@ -81,21 +72,7 @@ describe('[mutations] lifecycle', () => {
   });
 
   it('rebuilds the mutation on prop change when using `options`', done => {
-    const query = gql`
-      mutation addPerson {
-        allPeople(first: 1) {
-          people {
-            name
-          }
-        }
-      }
-    `;
-    const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
-    const networkInterface = mockNetworkInterface({
-      request: { query },
-      result: { data },
-    });
-    const client = new ApolloClient({ networkInterface, addTypename: false });
+    const client = createClient({});
 
     function options(props) {
       // expect(props.listId).toBe(2);
@@ -112,6 +89,7 @@ describe('[mutations] lifecycle', () => {
         return null;
       }
     }
+
     class ChangingProps extends React.Component<any, any> {
       state = { listId: 1 };
 
@@ -132,22 +110,7 @@ describe('[mutations] lifecycle', () => {
   });
 
   it('can execute a mutation with custom variables', done => {
-    const query = gql`
-      mutation addPerson($id: Int) {
-        allPeople(id: $id) {
-          people {
-            name
-          }
-        }
-      }
-    `;
-    const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
-    const variables = { id: 1 };
-    const networkInterface = mockNetworkInterface({
-      request: { query, variables },
-      result: { data },
-    });
-    const client = new ApolloClient({ networkInterface, addTypename: false });
+    const client = createClient({ id: 1 });
 
     @graphql(query)
     class Container extends React.Component<any, any> {
@@ -168,4 +131,37 @@ describe('[mutations] lifecycle', () => {
       </ApolloProvider>,
     );
   });
+
+  it('accepts client in options', done => {
+    const client = createClient({});
+
+    @graphql(query, { options: () => ({ client }) })
+    class Container extends React.Component<any, any> {
+      componentWillReceiveProps(props) {
+        if (props.listId !== 2) return;
+        props.mutate().then(x => done());
+      }
+      render() {
+        return null;
+      }
+    }
+
+    class ChangingProps extends React.Component<any, any> {
+      state = { listId: 1 };
+
+      componentDidMount() {
+        setTimeout(() => this.setState({ listId: 2 }), 50);
+      }
+
+      render() {
+        return <Container listId={this.state.listId} />;
+      }
+    }
+
+    renderer.create(
+      <ApolloProvider client={client}>
+        <ChangingProps />
+      </ApolloProvider>,
+    );
+  });
 });