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

Return undefined for result.data if no data is available #4356

Merged
merged 5 commits into from
Jan 31, 2019
Merged

Return undefined for result.data if no data is available #4356

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e79902
Tightened QueryManager getCurrentQueryResult typings
danilobuerger Jan 26, 2019
58232db
Added ObservableQuery getCurrentResult and deprecated currentResult
danilobuerger Jan 26, 2019
397139e
Use getCurrentResult instead of currentResult everywhere
danilobuerger Jan 26, 2019
ef46211
Merge branch 'master' into undefined
hwillson Jan 31, 2019
aaa9b92
Changelog update
hwillson Jan 31, 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
4 changes: 2 additions & 2 deletions docs/source/api/apollo-client.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ The `ApolloClient` class is the core API for Apollo, and the one you'll need to

{% tsapibox ObservableQuery.variables %}
{% tsapibox ObservableQuery.result %}
{% tsapibox ObservableQuery.currentResult %}
{% tsapibox ObservableQuery.getCurrentResult %}
{% tsapibox ObservableQuery.refetch %}
{% tsapibox ObservableQuery.setOptions %}
{% tsapibox ObservableQuery.setVariables %}
Expand All @@ -77,4 +77,4 @@ The `ApolloClient` class is the core API for Apollo, and the one you'll need to
{% tsapibox DefaultOptions %}
{% tsapibox NetworkStatus %}
{% tsapibox ApolloQueryResult %}
{% tsapibox ApolloCurrentResult %}
{% tsapibox ApolloCurrentQueryResult %}
4 changes: 3 additions & 1 deletion packages/apollo-client/src/__tests__/ApolloClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,9 @@ describe('ApolloClient', () => {
count++;
if (count === 1) {
expect(stripSymbols(result.data)).toEqual(data);
expect(stripSymbols(observable.currentResult().data)).toEqual(data);
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
data,
);
const bestFriends = result.data.people.friends.filter(
x => x.type === 'best',
);
Expand Down
35 changes: 30 additions & 5 deletions packages/apollo-client/src/core/ObservableQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {

import { QueryStoreValue } from '../data/queries';

// XXX remove in the next breaking semver change (3.0)
// Deprecated, use ApolloCurrentQueryResult
export type ApolloCurrentResult<T> = {
data: T | {};
errors?: ReadonlyArray<GraphQLError>;
Expand All @@ -29,6 +31,15 @@ export type ApolloCurrentResult<T> = {
partial?: boolean;
};

export type ApolloCurrentQueryResult<T> = {
data: T | undefined;
errors?: ReadonlyArray<GraphQLError>;
loading: boolean;
networkStatus: NetworkStatus;
error?: ApolloError;
partial?: boolean;
};

export interface FetchMoreOptions<
TData = any,
TVariables = OperationVariables
Expand Down Expand Up @@ -146,16 +157,30 @@ export class ObservableQuery<
});
}

// XXX remove in the next breaking semver change (3.0)
// Deprecated, use getCurrentResult()
public currentResult(): ApolloCurrentResult<TData> {
const result = this.getCurrentResult() as ApolloCurrentResult<TData>;
if (result.data === undefined) {
result.data = {};
}
return result;
}

/**
* Return the result of the query from the local cache as well as some fetching status
* `loading` and `networkStatus` allow to know if a request is in flight
* `partial` lets you know if the result from the local cache is complete or partial
* @return {data: Object, error: ApolloError, loading: boolean, networkStatus: number, partial: boolean}
*/
public currentResult(): ApolloCurrentResult<TData> {
public getCurrentResult(): ApolloCurrentQueryResult<TData> {
if (this.isTornDown) {
return {
data: this.lastError ? {} : this.lastResult ? this.lastResult.data : {},
data: this.lastError
? undefined
: this.lastResult
? this.lastResult.data
: undefined,
error: this.lastError,
loading: false,
networkStatus: NetworkStatus.error,
Expand All @@ -166,7 +191,7 @@ export class ObservableQuery<

if (hasError(queryStoreValue, this.options.errorPolicy)) {
return {
data: {},
data: undefined,
loading: false,
networkStatus: queryStoreValue.networkStatus,
error: new ApolloError({
Expand Down Expand Up @@ -220,7 +245,7 @@ export class ObservableQuery<
this.lastResultSnapshot = cloneDeep(this.lastResult);
}

return { ...result, partial } as ApolloCurrentResult<TData>;
return { ...result, partial };
}

// Compares newResult to the snapshot we took of this.lastResult when it was
Expand All @@ -237,7 +262,7 @@ export class ObservableQuery<
}

// Returns the last result that observer.next was called with. This is not the same as
// currentResult! If you're not sure which you need, then you probably need currentResult.
// getCurrentResult! If you're not sure which you need, then you probably need getCurrentResult.
public getLastResult(): ApolloQueryResult<TData> {
return this.lastResult;
}
Expand Down
20 changes: 12 additions & 8 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 @@ -967,7 +967,10 @@ export class QueryManager<TStore> {
public getCurrentQueryResult<T>(
observableQuery: ObservableQuery<T>,
optimistic: boolean = true,
) {
): {
data: T | undefined;
partial: boolean;
} {
const { variables, query } = observableQuery.options;
const lastResult = observableQuery.getLastResult();
const { newData } = this.getQuery(observableQuery.queryId);
Expand All @@ -977,16 +980,17 @@ export class QueryManager<TStore> {
} 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({
query,
variables,
previousResult: lastResult ? lastResult.data : undefined,
optimistic,
});
const data =
this.dataStore.getCache().read<T>({
query,
variables,
previousResult: lastResult ? lastResult.data : undefined,
optimistic,
}) || undefined;

return { data, partial: false };
} catch (e) {
return { data: {}, partial: true };
return { data: undefined, partial: true };
}
}
}
Expand Down
83 changes: 45 additions & 38 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 @@ -9,9 +9,8 @@ import mockQueryManager from '../../__mocks__/mockQueryManager';
import mockWatchQuery from '../../__mocks__/mockWatchQuery';
import { mockSingleLink } from '../../__mocks__/mockLinks';

import { ObservableQuery, ApolloCurrentResult } from '../ObservableQuery';
import { ObservableQuery } from '../ObservableQuery';
import { NetworkStatus } from '../networkStatus';
import { ApolloQueryResult, FetchType } from '../types';
import { QueryManager } from '../QueryManager';
import { DataStore } from '../../data/store';
import ApolloClient from '../../';
Expand Down Expand Up @@ -321,9 +320,9 @@ describe('ObservableQuery', () => {
expect(stripSymbols(result.data)).toEqual(data2);
// go back to first set of variables
observable.setOptions({ variables });
const current = observable.currentResult();
const current = observable.getCurrentResult();
expect(stripSymbols(current.data)).toEqual(data);
const secondCurrent = observable.currentResult();
const secondCurrent = observable.getCurrentResult();
expect(current.data).toEqual(secondCurrent.data);
done();
}
Expand Down Expand Up @@ -714,20 +713,20 @@ describe('ObservableQuery', () => {
subscribeAndCount(done, observable, (handleCount, result) => {
if (handleCount === 1) {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataOne,
);
observable.setVariables(differentVariables);
expect(stripSymbols(observable.currentResult().data)).toEqual({});
expect(observable.currentResult().loading).toBe(true);
expect(observable.getCurrentResult().data).toEqual(undefined);
expect(observable.getCurrentResult().loading).toBe(true);
}
// after loading is false and data has returned
if (handleCount === 3) {
expect(stripSymbols(result.data)).toEqual(dataTwo);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataTwo,
);
expect(observable.currentResult().loading).toBe(false);
expect(observable.getCurrentResult().loading).toBe(false);
done();
}
});
Expand Down Expand Up @@ -781,20 +780,20 @@ describe('ObservableQuery', () => {
subscribeAndCount(done, observable, (handleCount, result) => {
if (handleCount === 1) {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataOne,
);
observable.setVariables(differentVariables);
expect(observable.currentResult().data).toEqual({});
expect(observable.currentResult().loading).toBe(true);
expect(observable.getCurrentResult().data).toEqual(undefined);
expect(observable.getCurrentResult().loading).toBe(true);
}
// after loading is false and data has returned
if (handleCount === 3) {
expect(stripSymbols(result.data)).toEqual(dataTwo);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataTwo,
);
expect(observable.currentResult().loading).toBe(false);
expect(observable.getCurrentResult().loading).toBe(false);
done();
}
});
Expand Down Expand Up @@ -848,20 +847,20 @@ describe('ObservableQuery', () => {
subscribeAndCount(done, observable, (handleCount, result) => {
if (handleCount === 1) {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataOne,
);
observable.setVariables(differentVariables);
expect(observable.currentResult().data).toEqual({});
expect(observable.currentResult().loading).toBe(true);
expect(observable.getCurrentResult().data).toEqual(undefined);
expect(observable.getCurrentResult().loading).toBe(true);
}
// after loading is false and data has returned
if (handleCount === 3) {
expect(stripSymbols(result.data)).toEqual(dataTwo);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataTwo,
);
expect(observable.currentResult().loading).toBe(false);
expect(observable.getCurrentResult().loading).toBe(false);
done();
}
});
Expand All @@ -888,17 +887,17 @@ describe('ObservableQuery', () => {
subscribeAndCount(done, observable, (handleCount, result) => {
if (handleCount === 1) {
expect(result.errors).toEqual([error]);
expect(observable.currentResult().errors).toEqual([error]);
expect(observable.getCurrentResult().errors).toEqual([error]);
observable.setVariables(differentVariables);
expect(observable.currentResult().errors).toEqual([error]);
expect(observable.getCurrentResult().errors).toEqual([error]);
}
// after loading is done and new results are returned
if (handleCount === 3) {
expect(stripSymbols(result.data)).toEqual(dataTwo);
expect(stripSymbols(observable.currentResult().data)).toEqual(
expect(stripSymbols(observable.getCurrentResult().data)).toEqual(
dataTwo,
);
expect(observable.currentResult().loading).toBe(false);
expect(observable.getCurrentResult().loading).toBe(false);
done();
}
});
Expand Down Expand Up @@ -1387,7 +1386,7 @@ describe('ObservableQuery', () => {
});

subscribeAndCount(done, observable, (count, result) => {
const { data, loading, networkStatus } = observable.currentResult();
const { data, loading, networkStatus } = observable.getCurrentResult();
try {
expect(result).toEqual({
data,
Expand Down Expand Up @@ -1419,7 +1418,7 @@ describe('ObservableQuery', () => {
});

subscribeAndCount(done, observable, () => {
expect(stripSymbols(observable.currentResult())).toEqual({
expect(stripSymbols(observable.getCurrentResult())).toEqual({
data: dataOne,
loading: false,
networkStatus: 7,
Expand All @@ -1428,17 +1427,17 @@ describe('ObservableQuery', () => {
done();
});

expect(observable.currentResult()).toEqual({
expect(observable.getCurrentResult()).toEqual({
loading: true,
data: {},
data: undefined,
networkStatus: 1,
partial: true,
});
setTimeout(
wrap(done, () => {
expect(observable.currentResult()).toEqual({
expect(observable.getCurrentResult()).toEqual({
loading: true,
data: {},
data: undefined,
networkStatus: 1,
partial: true,
});
Expand All @@ -1464,7 +1463,7 @@ describe('ObservableQuery', () => {
query,
variables,
});
expect(stripSymbols(observable.currentResult())).toEqual({
expect(stripSymbols(observable.getCurrentResult())).toEqual({
data: dataOne,
loading: false,
networkStatus: 7,
Expand All @@ -1488,7 +1487,7 @@ describe('ObservableQuery', () => {
error: theError => {
expect(theError.graphQLErrors).toEqual([error]);

const currentResult = observable.currentResult();
const currentResult = observable.getCurrentResult();
expect(currentResult.loading).toBe(false);
expect(currentResult.error!.graphQLErrors).toEqual([error]);
done();
Expand All @@ -1510,10 +1509,10 @@ describe('ObservableQuery', () => {
return observable.result().catch((theError: any) => {
expect(theError.graphQLErrors).toEqual([error]);

const currentResult = observable.currentResult();
const currentResult = observable.getCurrentResult();
expect(currentResult.loading).toBe(false);
expect(currentResult.error!.graphQLErrors).toEqual([error]);
const currentResult2 = observable.currentResult();
const currentResult2 = observable.getCurrentResult();
expect(currentResult.error === currentResult2.error).toBe(true);
});
});
Expand All @@ -1533,7 +1532,7 @@ describe('ObservableQuery', () => {
return observable.result().then(result => {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(result.errors).toEqual([error]);
const currentResult = observable.currentResult();
const currentResult = observable.getCurrentResult();
expect(currentResult.loading).toBe(false);
expect(currentResult.errors).toEqual([error]);
expect(currentResult.error).toBeUndefined();
Expand All @@ -1555,7 +1554,7 @@ describe('ObservableQuery', () => {
return observable.result().then(result => {
expect(stripSymbols(result.data)).toEqual(dataOne);
expect(result.errors).toBeUndefined();
const currentResult = observable.currentResult();
const currentResult = observable.getCurrentResult();
expect(currentResult.loading).toBe(false);
expect(currentResult.errors).toBeUndefined();
expect(currentResult.error).toBeUndefined();
Expand All @@ -1580,15 +1579,19 @@ describe('ObservableQuery', () => {
variables,
fetchPolicy: 'network-only',
});
expect(stripSymbols(observable.currentResult())).toEqual({
expect(stripSymbols(observable.getCurrentResult())).toEqual({
data: dataOne,
loading: true,
networkStatus: 1,
partial: false,
});

subscribeAndCount(done, observable, (handleCount, subResult) => {
const { data, loading, networkStatus } = observable.currentResult();
const {
data,
loading,
networkStatus,
} = observable.getCurrentResult();
expect(subResult).toEqual({
data,
loading,
Expand Down Expand Up @@ -1650,7 +1653,11 @@ describe('ObservableQuery', () => {
});

subscribeAndCount(done, observable, (count, result) => {
const { data, loading, networkStatus } = observable.currentResult();
const {
data,
loading,
networkStatus,
} = observable.getCurrentResult();
expect(result).toEqual({
data,
loading,
Expand Down
Loading