Skip to content

Commit

Permalink
Document setVariables internal API status
Browse files Browse the repository at this point in the history
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 apollographql#2712
  • Loading branch information
PowerKiKi committed Jul 16, 2018
1 parent 9c36894 commit 8bf4bd0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 8 deletions.
7 changes: 4 additions & 3 deletions packages/apollo-client/src/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand Down
24 changes: 19 additions & 5 deletions packages/apollo-client/src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -432,7 +447,6 @@ export class ObservableQuery<
* this will refetch)
*
* @param fetchResults: Option to ignore fetching results when updating variables
*
*/
public setVariables(
variables: TVariables,
Expand Down
70 changes: 70 additions & 0 deletions packages/apollo-client/src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 8bf4bd0

Please sign in to comment.