Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support returnPartialData for watched queries again. #4743

Merged
merged 4 commits into from
Apr 24, 2019
Merged

Support returnPartialData for watched queries again. #4743

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bbe86c
Support returnPartialData for watched queries again.
benjamn Apr 22, 2019
c292dad
Restore returnPartialData tests.
benjamn Apr 22, 2019
21916e0
Bundle size tweaks.
benjamn Apr 23, 2019
23b4fe3
Mention PR #4743 in CHANGELOG.md.
benjamn Apr 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions packages/apollo-client/src/core/LocalState.ts
View file
Open in desktop
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);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwillson Do you think it's safe to remove this method if we're not using it internally?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamn Yes, for sure!


// 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ export class QueryManager<TStore> {
cancel,
}));

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

this.queryStore.initQuery({
queryId,
Expand Down Expand Up @@ -433,7 +433,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 @@ -454,7 +455,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 @@ -531,7 +533,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 @@ -549,15 +551,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 @@ -605,23 +611,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 @@ -790,7 +801,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 @@ -899,7 +910,7 @@ export class QueryManager<TStore> {
}

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

Expand Down Expand Up @@ -1018,31 +1029,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 @@ -1067,11 +1077,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 @@ -1228,7 +1235,8 @@ export class QueryManager<TStore> {
fetchMoreForQueryId,
);

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

this.broadcastQueries();
}
Expand All @@ -1248,17 +1256,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 @@ -1308,14 +1316,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
Open in desktop
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