Skip to content

Commit 63dd8c6

Browse files
benjamnbenjamn
committed
Read from cache again in ObservableQuery#getCurrentResult.
This partially reverts two commits from PR #5565, preserving related improvements that have happened in the meantime: * Revert "Remove partial field from ApolloCurrentQueryResult type." This reverts commit ca2cef6. * Revert "Stop reading from cache in ObservableQuery#getCurrentResult." This reverts commit 01376a3. One of the motivations behind the original changes was to lay the foundations for eventually supporting asynchronous (Promise-based) cache reads, but that form of asynchrony is no longer a goal of Apollo Client 3.0 or any planned future version, so there's not as much benefit to weakening getCurrentResult in this way. On the other hand, allowing getCurrentResult to read synchronously from the cache ensures that custom field read functions can return new results immediately, which helps with issues like #5782. Immediate cache reads have also been the behavior of Apollo Client for much longer than #5565, which makes this change appealing from an ease-of-upgrading perspective.
1 parent 617b1db commit 63dd8c6

File tree

4 files changed

+76
-30
lines changed

4 files changed

+76
-30
lines changed

‎src/core/ObservableQuery.ts

+34-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { isNonEmptyArray } from '../utilities/common/arrays';
2020

2121
export type ApolloCurrentQueryResult<T> = ApolloQueryResult<T> & {
2222
error?: ApolloError;
23+
partial?: boolean;
2324
};
2425

2526
export interface FetchMoreOptions<
@@ -130,9 +131,18 @@ export class ObservableQuery<
130131
});
131132
}
132133

134+
/**
135+
* Return the result of the query from the local cache as well as some fetching status
136+
* `loading` and `networkStatus` allow to know if a request is in flight
137+
* `partial` lets you know if the result from the local cache is complete or partial
138+
*/
133139
public getCurrentResult(): ApolloCurrentQueryResult<TData> {
134-
const { lastResult, lastError } = this;
135-
const { fetchPolicy } = this.options;
140+
const {
141+
lastResult,
142+
lastError,
143+
options: { fetchPolicy },
144+
} = this;
145+
136146
const isNetworkFetchPolicy =
137147
fetchPolicy === 'network-only' ||
138148
fetchPolicy === 'no-cache';
@@ -155,6 +165,9 @@ export class ObservableQuery<
155165
return result;
156166
}
157167

168+
const { data, partial } = this.queryManager.getCurrentQueryResult(this);
169+
Object.assign(result, { data, partial });
170+
158171
const queryStoreValue = this.queryManager.queryStore.get(this.queryId);
159172
if (queryStoreValue) {
160173
const { networkStatus } = queryStoreValue;
@@ -191,9 +204,27 @@ export class ObservableQuery<
191204
if (queryStoreValue.graphQLErrors && this.options.errorPolicy === 'all') {
192205
result.errors = queryStoreValue.graphQLErrors;
193206
}
207+
208+
} else {
209+
// We need to be careful about the loading state we show to the user, to try
210+
// and be vaguely in line with what the user would have seen from .subscribe()
211+
// but to still provide useful information synchronously when the query
212+
// will not end up hitting the server.
213+
// See more: https://github.com/apollostack/apollo-client/issues/707
214+
// Basically: is there a query in flight right now (modolo the next tick)?
215+
const loading = isNetworkFetchPolicy ||
216+
(partial && fetchPolicy !== 'cache-only');
217+
218+
Object.assign(result, {
219+
loading,
220+
networkStatus: loading ? NetworkStatus.loading : NetworkStatus.ready,
221+
});
194222
}
195223

196-
this.updateLastResult(result);
224+
if (!partial) {
225+
result.stale = false;
226+
this.updateLastResult(result);
227+
}
197228

198229
return result;
199230
}

‎src/core/QueryManager.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,7 @@ export class QueryManager<TStore> {
10521052
this.queries.delete(queryId);
10531053
}
10541054

1055-
private getCurrentQueryResult<T>(
1055+
public getCurrentQueryResult<T>(
10561056
observableQuery: ObservableQuery<T>,
10571057
optimistic: boolean = true,
10581058
): {

‎src/core/__tests__/ObservableQuery.ts

+39-25
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,7 @@ describe('ObservableQuery', () => {
14501450
loading: false,
14511451
networkStatus: 7,
14521452
stale: false,
1453+
partial: false,
14531454
});
14541455
resolve();
14551456
});
@@ -1459,6 +1460,7 @@ describe('ObservableQuery', () => {
14591460
data: undefined,
14601461
networkStatus: 1,
14611462
stale: false,
1463+
partial: true,
14621464
});
14631465

14641466
setTimeout(
@@ -1468,12 +1470,40 @@ describe('ObservableQuery', () => {
14681470
data: undefined,
14691471
networkStatus: 1,
14701472
stale: false,
1473+
partial: true,
14711474
});
14721475
}),
14731476
0,
14741477
);
14751478
});
14761479

1480+
itAsync('returns results from the store immediately', (resolve, reject) => {
1481+
const queryManager = mockQueryManager(reject, {
1482+
request: { query, variables },
1483+
result: { data: dataOne },
1484+
});
1485+
1486+
return queryManager.query({ query, variables }).then((result: any) => {
1487+
expect(stripSymbols(result)).toEqual({
1488+
data: dataOne,
1489+
loading: false,
1490+
networkStatus: 7,
1491+
stale: false,
1492+
});
1493+
const observable = queryManager.watchQuery({
1494+
query,
1495+
variables,
1496+
});
1497+
expect(stripSymbols(observable.getCurrentResult())).toEqual({
1498+
data: dataOne,
1499+
loading: true,
1500+
networkStatus: NetworkStatus.loading,
1501+
stale: false,
1502+
partial: false,
1503+
});
1504+
}).then(resolve, reject);
1505+
});
1506+
14771507
itAsync('returns errors from the store immediately', (resolve, reject) => {
14781508
const queryManager = mockQueryManager(reject, {
14791509
request: { query, variables },
@@ -1563,7 +1593,7 @@ describe('ObservableQuery', () => {
15631593
}).then(resolve, reject);
15641594
});
15651595

1566-
itAsync('returns partial data from the store', (resolve, reject) => {
1596+
itAsync('returns partial data from the store immediately', (resolve, reject) => {
15671597
const superQuery = gql`
15681598
query superQuery($id: ID!) {
15691599
people_one(id: $id) {
@@ -1600,10 +1630,11 @@ describe('ObservableQuery', () => {
16001630
});
16011631

16021632
expect(observable.getCurrentResult()).toEqual({
1603-
data: void 0,
1633+
data: dataOne,
16041634
loading: true,
16051635
networkStatus: 1,
16061636
stale: false,
1637+
partial: true,
16071638
});
16081639

16091640
// we can use this to trigger the query
@@ -1618,28 +1649,19 @@ describe('ObservableQuery', () => {
16181649

16191650
if (handleCount === 1) {
16201651
expect(subResult).toEqual({
1621-
data: void 0,
1652+
data: dataOne,
16221653
loading: true,
16231654
networkStatus: 1,
16241655
stale: false,
16251656
});
16261657

16271658
} else if (handleCount === 2) {
1628-
expect(subResult).toEqual({
1629-
data: dataOne,
1630-
loading: false,
1631-
networkStatus: 7,
1632-
stale: false,
1633-
});
1634-
1635-
} else if (handleCount === 3) {
16361659
expect(subResult).toEqual({
16371660
data: superDataOne,
16381661
loading: false,
16391662
networkStatus: 7,
16401663
stale: false,
16411664
});
1642-
16431665
resolve();
16441666
}
16451667
});
@@ -1670,6 +1692,7 @@ describe('ObservableQuery', () => {
16701692
loading: true,
16711693
networkStatus: 1,
16721694
stale: false,
1695+
partial: false,
16731696
});
16741697

16751698
subscribeAndCount(reject, observable, (handleCount, subResult) => {
@@ -1678,30 +1701,20 @@ describe('ObservableQuery', () => {
16781701
loading,
16791702
networkStatus,
16801703
} = observable.getCurrentResult();
1681-
16821704
expect(subResult).toEqual({
16831705
data,
16841706
loading,
16851707
networkStatus,
16861708
stale: false,
16871709
});
16881710

1689-
if (handleCount === 1) {
1690-
expect(stripSymbols(subResult)).toEqual({
1691-
data: void 0,
1692-
loading: true,
1693-
networkStatus: NetworkStatus.loading,
1694-
stale: false,
1695-
});
1696-
1697-
} else if (handleCount === 2) {
1711+
if (handleCount === 2) {
16981712
expect(stripSymbols(subResult)).toEqual({
16991713
data: dataTwo,
17001714
loading: false,
1701-
networkStatus: NetworkStatus.ready,
1715+
networkStatus: 7,
17021716
stale: false,
17031717
});
1704-
17051718
resolve();
17061719
}
17071720
});
@@ -1727,12 +1740,12 @@ describe('ObservableQuery', () => {
17271740
variables,
17281741
fetchPolicy: 'no-cache',
17291742
});
1730-
17311743
expect(stripSymbols(observable.getCurrentResult())).toEqual({
17321744
data: undefined,
17331745
loading: true,
17341746
networkStatus: 1,
17351747
stale: false,
1748+
partial: false,
17361749
});
17371750

17381751
subscribeAndCount(reject, observable, (handleCount, subResult) => {
@@ -1748,6 +1761,7 @@ describe('ObservableQuery', () => {
17481761
loading,
17491762
networkStatus,
17501763
stale: false,
1764+
partial: false,
17511765
});
17521766
} else if (handleCount === 2) {
17531767
expect(stripSymbols(subResult)).toEqual({

‎src/react/data/QueryData.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ export class QueryData<TData, TVariables> extends OperationData {
353353
} else {
354354
// Fetch the current result (if any) from the store.
355355
const currentResult = this.currentObservable.query!.getCurrentResult();
356-
const { loading, networkStatus, errors } = currentResult;
356+
const { loading, partial, networkStatus, errors } = currentResult;
357357
let { error, data } = currentResult;
358358

359359
// Until a set naming convention for networkError and graphQLErrors is
@@ -390,6 +390,7 @@ export class QueryData<TData, TVariables> extends OperationData {
390390
const { partialRefetch } = options;
391391
if (
392392
partialRefetch &&
393+
partial &&
393394
(!data || Object.keys(data).length === 0) &&
394395
fetchPolicy !== 'cache-only'
395396
) {

0 commit comments

Comments
 (0)