From adc22f9c59eef055f115ccd0bb2819d438a28041 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Mon, 16 Jul 2018 15:48:16 +0900 Subject: [PATCH] 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 | 2 + packages/apollo-client/src/ApolloClient.ts | 7 +- .../apollo-client/src/core/ObservableQuery.ts | 24 +++++-- .../src/core/__tests__/ObservableQuery.ts | 70 +++++++++++++++++++ 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/packages/apollo-client/CHANGELOG.md b/packages/apollo-client/CHANGELOG.md index 108c5446a6c..deec67b92b5 100644 --- a/packages/apollo-client/CHANGELOG.md +++ b/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 diff --git a/packages/apollo-client/src/ApolloClient.ts b/packages/apollo-client/src/ApolloClient.ts index 4f926296a04..bfd7227a421 100644 --- a/packages/apollo-client/src/ApolloClient.ts +++ b/packages/apollo-client/src/ApolloClient.ts @@ -186,9 +186,9 @@ export default class ApolloClient 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. *

* 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 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. *

+ * Note that if the cache does not change, the subscriber will *not* be notified. + *

* See [here](https://medium.com/apollo-stack/the-concepts-of-graphql-bc68bd819be3#.3mb0cbcmc) for * a description of store reactivity. - * */ public watchQuery(options: WatchQueryOptions): ObservableQuery { this.initQueryManager(); diff --git a/packages/apollo-client/src/core/ObservableQuery.ts b/packages/apollo-client/src/core/ObservableQuery.ts index 3df67f34a35..e0f1b03884a 100644 --- a/packages/apollo-client/src/core/ObservableQuery.ts +++ b/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> { 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, diff --git a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts index 3b58565725d..df19a96c3c1 100644 --- a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts +++ b/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 = 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', () => {