Skip to content

Commit

Permalink
Merge pull request apollographql#4743 from apollographql/support-retu…
Browse files Browse the repository at this point in the history 
…rnPartialData-again

Support returnPartialData for watched queries again.
  • Loading branch information
@benjamn
benjamn authored Apr 24, 2019
2 parents 82e7530 + 23b4fe3 commit d2787e4
Show file tree
Hide file tree
Showing 9 changed files with 496 additions and 134 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
- Accommodate `@client @export` variable changes in `ObservableQuery`. <br/>
[@hwillson](https://github.com/hwillson) in [#4604](https://github.com/apollographql/apollo-client/pull/4604)

- Support the `returnPartialData` option for watched queries again. <br/>
[@benjamn](https://github.com/benjamn) in [#4743](https://github.com/apollographql/apollo-client/pull/4743)

### Apollo Cache In-Memory

- Support `new InMemoryCache({ freezeResults: true })` to help enforce immutability. <br/>
Expand Down
7 changes: 2 additions & 5 deletions packages/apollo-client/src/core/LocalState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,6 @@ export class LocalState<TCacheShape> {
return forceResolvers;
}

public shouldForceResolver(field: FieldNode) {
return this.shouldForceResolvers(field);
}

// Query the cache and return matching data.
private buildRootValueFromCache(
document: DocumentNode,
Expand All @@ -262,6 +258,7 @@ export class LocalState<TCacheShape> {
return this.cache.diff({
query: buildQueryFromSelectionSet(document),
variables,
returnPartialData: true,
optimistic: false,
}).result;
}
Expand Down Expand Up @@ -383,7 +380,7 @@ export class LocalState<TCacheShape> {
// `@client(always: true)`), then we'll skip running non-forced resolvers.
if (
!execContext.onlyRunForcedResolvers ||
this.shouldForceResolver(field)
this.shouldForceResolvers(field)
) {
const resolverType =
rootValue.__typename || execContext.defaultOperationType;
Expand Down
125 changes: 65 additions & 60 deletions packages/apollo-client/src/core/QueryManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ export class QueryManager<TStore> {
cancel,
}));

this.invalidate(true, fetchMoreForQueryId);
this.invalidate(fetchMoreForQueryId);

this.queryStore.initQuery({
queryId,
Expand Down Expand Up @@ -429,7 +429,8 @@ export class QueryManager<TStore> {
} else {
if (requestId >= this.getQuery(queryId).lastRequestId) {
this.queryStore.markQueryError(queryId, error, fetchMoreForQueryId);
this.invalidate(true, queryId, fetchMoreForQueryId);
this.invalidate(queryId);
this.invalidate(fetchMoreForQueryId);
this.broadcastQueries();
}
throw new ApolloError({ networkError: error });
Expand All @@ -450,7 +451,8 @@ export class QueryManager<TStore> {
// If there is no part of the query we need to fetch from the server (or,
// fetchPolicy is cache-only), we just write the store result as the final result.
this.queryStore.markQueryResultClient(queryId, !shouldFetch);
this.invalidate(true, queryId, fetchMoreForQueryId);
this.invalidate(queryId);
this.invalidate(fetchMoreForQueryId);

if (this.transform(query).hasForcedResolvers) {
return this.localState.runResolvers({
Expand Down Expand Up @@ -527,7 +529,7 @@ export class QueryManager<TStore> {
newData?: Cache.DiffResult<T>,
) => {
// we're going to take a look at the data, so the query is no longer invalidated
this.invalidate(false, queryId);
this.invalidate(queryId, false);

// The query store value can be undefined in the event of a store
// reset.
Expand All @@ -545,15 +547,19 @@ export class QueryManager<TStore> {
const loading = isNetworkRequestInFlight(queryStoreValue.networkStatus);
const lastResult = observableQuery && observableQuery.getLastResult();

if (
loading &&
fetchPolicy !== 'cache-only' &&
fetchPolicy !== 'cache-and-network' &&
(newData || !queryStoreValue.previousVariables) &&
!(lastResult &&
lastResult.networkStatus !== queryStoreValue.networkStatus &&
options.notifyOnNetworkStatusChange)
) {
const networkStatusChanged = !!(
lastResult &&
lastResult.networkStatus !== queryStoreValue.networkStatus
);

const shouldNotifyIfLoading =
options.returnPartialData ||
(!newData && queryStoreValue.previousVariables) ||
(networkStatusChanged && options.notifyOnNetworkStatusChange) ||
fetchPolicy === 'cache-only' ||
fetchPolicy === 'cache-and-network';

if (loading && !shouldNotifyIfLoading) {
return;
}

Expand Down Expand Up @@ -601,23 +607,28 @@ export class QueryManager<TStore> {
data = lastResult.data;
isMissing = false;
} else {
const readResult = this.dataStore.getCache().diff({
const diffResult = this.dataStore.getCache().diff({
query: document as DocumentNode,
variables:
queryStoreValue.previousVariables ||
queryStoreValue.variables,
returnPartialData: true,
optimistic: true,
});

data = readResult.result;
isMissing = !readResult.complete;
data = diffResult.result;
isMissing = !diffResult.complete;
}
}

// If there is some data missing and the user has told us that they
// do not tolerate partial data then we want to return the previous
// result and mark it as stale.
const stale = isMissing && fetchPolicy !== 'cache-only';
const stale = isMissing && !(
options.returnPartialData ||
fetchPolicy === 'cache-only'
);

const resultFromStore: ApolloQueryResult<T> = {
data: stale ? lastResult && lastResult.data : data,
loading,
Expand Down Expand Up @@ -786,7 +797,7 @@ export class QueryManager<TStore> {
private stopQueryInStoreNoBroadcast(queryId: string) {
this.stopPollingQuery(queryId);
this.queryStore.stopQuery(queryId);
this.invalidate(true, queryId);
this.invalidate(queryId);
}

public addQueryListener(queryId: string, listener: QueryListener) {
Expand Down Expand Up @@ -895,7 +906,7 @@ export class QueryManager<TStore> {
}

this.setQuery(queryId, () => ({ newData: null }));
this.invalidate(true, queryId);
this.invalidate(queryId);
}
});

Expand Down Expand Up @@ -1014,31 +1025,30 @@ export class QueryManager<TStore> {
data: T | undefined;
partial: boolean;
} {
const { variables, query, fetchPolicy } = observableQuery.options;
const { variables, query, fetchPolicy, returnPartialData } = observableQuery.options;
const lastResult = observableQuery.getLastResult();
const { newData } = this.getQuery(observableQuery.queryId);

// XXX test this
if (newData && newData.complete) {
return { data: newData.result, partial: false };
} else if (fetchPolicy === 'no-cache' || fetchPolicy === 'network-only') {
return { data: undefined, partial: false };
} else {
try {
// the query is brand new, so we read from the store to see if anything is there
const data =
this.dataStore.getCache().read<T>({
query,
variables,
previousResult: lastResult ? lastResult.data : undefined,
optimistic,
}) || undefined;
}

return { data, partial: false };
} catch (e) {
return { data: undefined, partial: true };
}
if (fetchPolicy === 'no-cache' || fetchPolicy === 'network-only') {
return { data: undefined, partial: false };
}

const { result, complete } = this.dataStore.getCache().diff<T>({
query,
variables,
previousResult: lastResult ? lastResult.data : undefined,
returnPartialData: true,
optimistic,
});

return {
data: (complete || returnPartialData) ? result : void 0,
partial: !complete,
};
}

public getQueryWithPreviousResult<TData, TVariables = OperationVariables>(
Expand All @@ -1063,11 +1073,8 @@ export class QueryManager<TStore> {
}

const { variables, query } = observableQuery.options;

const { data } = this.getCurrentQueryResult(observableQuery, false);

return {
previousResult: data,
previousResult: this.getCurrentQueryResult(observableQuery, false).data,
variables,
document: query,
};
Expand Down Expand Up @@ -1224,7 +1231,8 @@ export class QueryManager<TStore> {
fetchMoreForQueryId,
);

this.invalidate(true, queryId, fetchMoreForQueryId);
this.invalidate(queryId);
this.invalidate(fetchMoreForQueryId);

this.broadcastQueries();
}
Expand All @@ -1244,17 +1252,17 @@ export class QueryManager<TStore> {
// the original result in case an @connection directive is used.
resultFromStore = result.data;
} else {
try {
// ensure result is combined with data already in store
resultFromStore = this.dataStore.getCache().read({
variables,
query: document,
optimistic: false,
});
// this will throw an error if there are missing fields in
// the results which can happen with errors from the server.
// tslint:disable-next-line
} catch (e) {}
// ensure result is combined with data already in store
const { result, complete } = this.dataStore.getCache().diff<T>({
variables,
query: document,
optimistic: false,
returnPartialData: true,
});

if (complete || options.returnPartialData) {
resultFromStore = result;
}
}
}).subscribe({
error(error: ApolloError) {
Expand Down Expand Up @@ -1304,14 +1312,11 @@ export class QueryManager<TStore> {
}

private invalidate(
invalidated: boolean,
queryId?: string,
fetchMoreForQueryId?: string,
queryId: string | undefined,
invalidated = true,
) {
if (queryId) this.setQuery(queryId, () => ({ invalidated }));

if (fetchMoreForQueryId) {
this.setQuery(fetchMoreForQueryId, () => ({ invalidated }));
if (queryId) {
this.setQuery(queryId, () => ({ invalidated }));
}
}

Expand Down
82 changes: 78 additions & 4 deletions packages/apollo-client/src/core/__tests__/ObservableQuery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,14 +698,14 @@ describe('ObservableQuery', () => {
},
);

subscribeAndCount(done, observable, (handleCount, result) => {
subscribeAndCount(done, observable, async (handleCount, result) => {
if (handleCount === 1) {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataOne,
);
observable.setVariables(differentVariables);
expect(observable.getCurrentResult().data).toEqual(undefined);
await observable.setVariables(differentVariables);
expect(observable.getCurrentResult().data).toEqual({});
expect(observable.getCurrentResult().loading).toBe(true);
}
// after loading is false and data has returned
Expand Down Expand Up @@ -772,7 +772,7 @@ describe('ObservableQuery', () => {
dataOne,
);
await observable.setVariables(differentVariables);
expect(observable.getCurrentResult().data).toEqual(undefined);
expect(observable.getCurrentResult().data).toEqual({});
expect(observable.getCurrentResult().loading).toBe(true);
}
// after loading is false and data has returned
Expand Down Expand Up @@ -1354,6 +1354,7 @@ describe('ObservableQuery', () => {
networkStatus: 1,
partial: true,
});

setTimeout(
wrap(done, () => {
expect(observable.getCurrentResult()).toEqual({
Expand Down Expand Up @@ -1482,6 +1483,79 @@ describe('ObservableQuery', () => {
});
});

it('returns partial data from the store immediately', done => {
const superQuery = gql`
query superQuery($id: ID!) {
people_one(id: $id) {
name
age
}
}
`;

const superDataOne = {
people_one: {
name: 'Luke Skywalker',
age: 21,
},
};

const queryManager = mockQueryManager(
{
request: { query, variables },
result: { data: dataOne },
},
{
request: { query: superQuery, variables },
result: { data: superDataOne },
},
);

queryManager.query({ query, variables }).then(result => {
const observable = queryManager.watchQuery({
query: superQuery,
variables,
returnPartialData: true,
});

expect(observable.currentResult()).toEqual({
data: dataOne,
loading: true,
networkStatus: 1,
partial: true,
});

// we can use this to trigger the query
subscribeAndCount(done, observable, (handleCount, subResult) => {
const { data, loading, networkStatus } = observable.currentResult();
expect(subResult).toEqual({
data,
loading,
networkStatus,
stale: false,
});

if (handleCount === 1) {
expect(subResult).toEqual({
data: dataOne,
loading: true,
networkStatus: 1,
stale: false,
});

} else if (handleCount === 2) {
expect(subResult).toEqual({
data: superDataOne,
loading: false,
networkStatus: 7,
stale: false,
});
done();
}
});
});
});

it('returns loading even if full data is available when using network-only fetchPolicy', done => {
const queryManager = mockQueryManager(
{
Expand Down
Loading

0 comments on commit d2787e4

Please sign in to comment.