Document setVariables internal API status

This clarifies `setVariables` as for internal use only because it has
a severe caveat where a result coming from cache will not notify subscribers.
That makes it a poor choice for most common use cases.

Instead `refetch` should be used and is now documented as such.

Also clarifies that `watchQuery` does not watch the result, but the cache store,
meaning that if the store does not change, the subcribers will not be notified.
In practice that means that a result coming from cache will never be seen by
subscribers.

This partially address #2712

packages/apollo-client/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - Updated `graphql` `peerDependencies` to handle 14.x versions.  
   [PR #3598](https://github.com/apollographql/apollo-client/pull/3598)
+- Document `setVariables` internal API status.
+  [PR #3692](https://github.com/apollographql/apollo-client/pull/3692)
 
 ### 2.3.5
 

packages/apollo-client/src/ApolloClient.ts

@@ -186,9 +186,9 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
     this.version = version;
   }
   /**
-   * This watches the results of the query according to the options specified and
+   * This watches the cache store of the query according to the options specified and
    * returns an {@link ObservableQuery}. We can subscribe to this {@link ObservableQuery} and
-   * receive updated results through a GraphQL observer.
+   * receive updated results through a GraphQL observer when the cache store changes.
    * <p /><p />
    * Note that this method is not an implementation of GraphQL subscriptions. Rather,
    * it uses Apollo's store in order to reactively deliver updates to your query results.
@@ -199,9 +199,10 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
    * first and last name and his/her first name has now changed. Then, any observers associated
    * with the results of the first query will be updated with a new result object.
    * <p /><p />
+   * Note that if the cache does not change, the subscriber will *not* be notified.
+   * <p /><p />
    * See [here](https://medium.com/apollo-stack/the-concepts-of-graphql-bc68bd819be3#.3mb0cbcmc) for
    * a description of store reactivity.
-   *
    */
   public watchQuery<T>(options: WatchQueryOptions): ObservableQuery<T> {
     this.initQueryManager();

packages/apollo-client/src/core/ObservableQuery.ts

@@ -242,6 +242,13 @@ export class ObservableQuery<
     this.isTornDown = false;
   }
 
+  /**
+   * Update the variables of this observable query, and fetch the new results.
+   * This method should be preferred over `setVariables` in most use cases.
+   *
+   * @param variables: The new set of variables. If there are missing variables,
+   * the previous values of those variables will be used.
+   */
   public refetch(variables?: TVariables): Promise<ApolloQueryResult<TData>> {
     const { fetchPolicy } = this.options;
     // early return if trying to read from cache during refetch
@@ -415,14 +422,22 @@ export class ObservableQuery<
   }
 
   /**
+   * This is for *internal* use only. Most users should instead use `refetch`
+   * in order to be properly notified of results even when they come from cache.
+   *
    * Update the variables of this observable query, and fetch the new results
    * if they've changed. If you want to force new results, use `refetch`.
    *
-   * Note: if the variables have not changed, the promise will return the old
-   * results immediately, and the `next` callback will *not* fire.
+   * Note: the `next` callback will *not* fire if the variables have not changed
+   * or if the result is coming from cache.
+   *
+   * Note: the promise will return the old results immediately if the variables
+   * have not changed.
    *
-   * Note: if the query is not active (there are no subscribers), the promise
-   * will return null immediately.
+   * Note: the promise will return null immediately if the query is not active
+   * (there are no subscribers).
+   *
+   * @private
    *
    * @param variables: The new set of variables. If there are missing variables,
    * the previous values of those variables will be used.
@@ -432,7 +447,6 @@ export class ObservableQuery<
    * this will refetch)
    *
    * @param fetchResults: Option to ignore fetching results when updating variables
-   *
    */
   public setVariables(
     variables: TVariables,

packages/apollo-client/src/core/__tests__/ObservableQuery.ts

@@ -1240,6 +1240,76 @@ describe('ObservableQuery', () => {
         }
       });
     });
+
+    it('calls ObservableQuery.next even after hitting cache', done => {
+      // This query and variables are copied from react-apollo
+      const queryWithVars = gql`
+        query people($first: Int) {
+          allPeople(first: $first) {
+            people {
+              name
+            }
+          }
+        }
+      `;
+
+      const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } };
+      const variables1 = { first: 0 };
+
+      const data2 = { allPeople: { people: [{ name: 'Leia Skywalker' }] } };
+      const variables2 = { first: 1 };
+
+      const observable: ObservableQuery<any> = mockWatchQuery(
+        {
+          request: {
+            query: queryWithVars,
+            variables: variables1,
+          },
+          result: { data },
+        },
+        {
+          request: {
+            query: queryWithVars,
+            variables: variables2,
+          },
+          result: { data: data2 },
+        },
+        {
+          request: {
+            query: queryWithVars,
+            variables: variables1,
+          },
+          result: { data },
+        },
+      );
+
+      observable.setOptions({ fetchPolicy: 'cache-and-network' });
+
+      subscribeAndCount(done, observable, (handleCount, result) => {
+        if (handleCount === 1) {
+          expect(result.data).toBeUndefined();
+          expect(result.loading).toBe(true);
+        } else if (handleCount === 2) {
+          expect(stripSymbols(result.data)).toEqual(data);
+          expect(result.loading).toBe(false);
+          observable.refetch(variables2);
+        } else if (handleCount === 3) {
+          expect(stripSymbols(result.data)).toEqual(data);
+          expect(result.loading).toBe(true);
+        } else if (handleCount === 4) {
+          expect(stripSymbols(result.data)).toEqual(data2);
+          expect(result.loading).toBe(false);
+          observable.refetch(variables1);
+        } else if (handleCount === 5) {
+          expect(stripSymbols(result.data)).toEqual(data2);
+          expect(result.loading).toBe(true);
+        } else if (handleCount === 6) {
+          expect(stripSymbols(result.data)).toEqual(data);
+          expect(result.loading).toBe(false);
+          done();
+        }
+      });
+    });
   });
 
   describe('currentResult', () => {