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.

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

Local fetch option in WatchQueryOptions #385

Merged
merged 13 commits into from
Jul 14, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Expect active development and potentially significant breaking changes in the `0

### vNEXT

- Added a "noFetch" option to WatchQueryOptions that only returns available data from the local store (even it is incomplete). [Issue #225] https://github.com/apollostack/apollo-client/issues/225 and [PR #385]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a valid markdown link :]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

- **Breaking change** Moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery. This shouldn't affect anyone using `react-apollo`, but if you were calling those methods on the subscription directly, you need to call them on the query handle/observable instead. The benefit of this is that developers that want to use RxJS for their observable handling can now have access to these methods. [Issue #194] (https://github.com/apollostack/apollo-client/issues/194) and [PR #362] (https://github.com/apollostack/apollo-client/pull/362)
- **Breaking change** Unified error handling for GraphQL errors and network errors. Both now result in rejected promises and passed as errors on observables through a new `ApolloError` type. This is a significant departure from the previous method of error handling which passed GraphQL errors in resolvers and `next` methods on subscriptions. [PR #352](https://github.com/apollostack/apollo-client/pull/352)

Expand Down
17 changes: 13 additions & 4 deletions src/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export interface WatchQueryOptions {
variables?: { [key: string]: any };
forceFetch?: boolean;
returnPartialData?: boolean;
noFetch?: boolean;
pollInterval?: number;
fragments?: FragmentDefinition[];
}
Expand Down Expand Up @@ -308,6 +309,7 @@ export class QueryManager {
selectionSet: queryStoreValue.query.selectionSet,
variables: queryStoreValue.variables,
returnPartialData: options.returnPartialData,
noFetch: options.noFetch,
fragmentMap: queryStoreValue.fragmentMap,
});

Expand Down Expand Up @@ -355,6 +357,7 @@ export class QueryManager {
this.queryListenerForObserver(options, observer)
);


return retQuerySubscription;
};

Expand All @@ -364,7 +367,7 @@ export class QueryManager {

// Use the same options as before, but with new variables and forceFetch true
return this.fetchQuery(queryId, assign(options, {
forceFetch: true,
forceFetch: options.noFetch ? false : true,
variables,
}) as WatchQueryOptions);
};
Expand All @@ -379,7 +382,7 @@ export class QueryManager {
this.pollingTimers[queryId] = setInterval(() => {
const pollingOptions = assign({}, options) as WatchQueryOptions;
// subsequent fetches from polling always reqeust new data
pollingOptions.forceFetch = true;
pollingOptions.forceFetch = options.noFetch ? false : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing this, we should not start the polling timer. I think if someone passed noFetch for their query we should error on anything that would initiate a fetch to the server, rather than ignoring it silently.

this.fetchQuery(queryId, pollingOptions);
}, pollInterval);
};
Expand Down Expand Up @@ -520,6 +523,7 @@ export class QueryManager {
variables,
forceFetch = false,
returnPartialData = false,
noFetch = false,
fragments = [],
} = options;

Expand Down Expand Up @@ -566,7 +570,7 @@ export class QueryManager {

initialResult = result;

if (missingSelectionSets && missingSelectionSets.length) {
if (missingSelectionSets && missingSelectionSets.length && !noFetch) {
const diffedQuery = queryDocument({
missingSelectionSets,
variableDefinitions: queryDef.variableDefinitions,
Expand Down Expand Up @@ -602,12 +606,13 @@ export class QueryManager {
variables,
forceFetch,
returnPartialData,
noFetch,
queryId,
requestId,
fragmentMap: queryFragmentMap,
});

if (! minimizedQuery || returnPartialData) {
if (! minimizedQuery || returnPartialData || noFetch) {
this.store.dispatch({
type: 'APOLLO_QUERY_RESULT_CLIENT',
result: {
Expand Down Expand Up @@ -666,6 +671,7 @@ export class QueryManager {
selectionSet: querySS.selectionSet,
variables,
returnPartialData: returnPartialData,
noFetch: noFetch,
fragmentMap: queryFragmentMap,
});
// ensure multiple errors don't get thrown
Expand Down Expand Up @@ -701,6 +707,9 @@ export class QueryManager {

private startQuery(queryId: string, options: WatchQueryOptions, listener: QueryListener) {
this.queryListeners[queryId] = listener;
if (options.noFetch) {
options.pollInterval = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

If someone passes noFetch and a pollInterval, I assume we should throw an error rather than silently ignoring it.

}
this.fetchQuery(queryId, options);

if (options.pollInterval) {
Expand Down
1 change: 1 addition & 0 deletions src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface QueryInitAction {
variables: Object;
forceFetch: boolean;
returnPartialData: boolean;
noFetch: boolean;
queryId: string;
requestId: number;
fragmentMap: FragmentMap;
Expand Down
10 changes: 9 additions & 1 deletion src/data/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ export function readQueryFromStore({
query,
variables,
returnPartialData,
noFetch,
}: {
store: NormalizedCache,
query: Document,
variables?: Object,
returnPartialData?: boolean,
noFetch?: boolean,
}): Object {
const queryDef = getQueryDefinition(query);

Expand All @@ -40,6 +42,7 @@ export function readQueryFromStore({
selectionSet: queryDef.selectionSet,
variables,
returnPartialData,
noFetch,
});
}

Expand All @@ -49,12 +52,14 @@ export function readFragmentFromStore({
rootId,
variables,
returnPartialData,
noFetch,
}: {
store: NormalizedCache,
fragment: Document,
rootId: string,
variables?: Object,
returnPartialData?: boolean,
noFetch?: boolean,
}): Object {
const fragmentDef = getFragmentDefinition(fragment);

Expand All @@ -64,6 +69,7 @@ export function readFragmentFromStore({
selectionSet: fragmentDef.selectionSet,
variables,
returnPartialData,
noFetch,
});
}

Expand All @@ -73,13 +79,15 @@ export function readSelectionSetFromStore({
selectionSet,
variables,
returnPartialData = false,
noFetch = false,
fragmentMap,
}: {
store: NormalizedCache,
rootId: string,
selectionSet: SelectionSet,
variables: Object,
returnPartialData?: boolean,
noFetch?: boolean,
fragmentMap?: FragmentMap,
}): Object {
const {
Expand All @@ -88,7 +96,7 @@ export function readSelectionSetFromStore({
selectionSet,
rootId,
store,
throwOnMissingField: !returnPartialData,
throwOnMissingField: !returnPartialData && !noFetch,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to pass through noFetch everywhere just to do this? I feel like we could just set returnPartialData: true in QueryManager or something instead of adding a new option.

variables,
fragmentMap,
});
Expand Down
2 changes: 2 additions & 0 deletions src/queries/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface QueryStoreValue {
graphQLErrors: GraphQLError[];
forceFetch: boolean;
returnPartialData: boolean;
noFetch: boolean;
lastRequestId: number;
fragmentMap: FragmentMap;
}
Expand Down Expand Up @@ -70,6 +71,7 @@ export function queries(
graphQLErrors: null,
forceFetch: action.forceFetch,
returnPartialData: action.returnPartialData,
noFetch: action.noFetch,
lastRequestId: action.requestId,
fragmentMap: action.fragmentMap,
};
Expand Down
144 changes: 144 additions & 0 deletions test/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,150 @@ describe('QueryManager', () => {
});
});

it('supports noFetch fetching only cached data', () => {
const primeQuery = gql`
query primeQuery {
luke: people_one(id: 1) {
name
}
}
`;

const complexQuery = gql`
query complexQuery {
luke: people_one(id: 1) {
name
}
vader: people_one(id: 4) {
name
}
}
`;

const data1 = {
luke: {
name: 'Luke Skywalker',
},
};

const data2 = {
luke: {
name: 'Luke Skywalker',
},
vader: {
name: 'Darth Vader',
},
};

const networkInterface = mockNetworkInterface(
{
request: { query: primeQuery },
result: { data: data1 },
},
{
request: { query: complexQuery },
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are trying to ensure that the query won't be fetched, we should remove this from the network mock.

result: { data: data2 },
}
);

const queryManager = new QueryManager({
networkInterface,
store: createApolloStore(),
reduxRootKey: 'apollo',
});

// First, prime the cache
queryManager.query({
query: primeQuery,
}).then(() => {
const handle = queryManager.watchQuery({
query: complexQuery,
noFetch: true,
});

return handle.result().then((result) => {
assert.equal(result.data['luke'].name, 'Luke Skywalker');
assert.notProperty(result.data, 'vader');
});
});
});

it('supports noFetch refetching only cached data', () => {
const primeQuery = gql`
query primeQuery {
luke: people_one(id: 1) {
name
}
}
`;

const complexQuery = gql`
query complexQuery {
luke: people_one(id: 1) {
name
}
vader: people_one(id: 4) {
name
}
}
`;

const data1 = {
luke: {
name: 'Luke Skywalker',
},
};

const data2 = {
luke: {
name: 'Luke Skywalker',
},
vader: {
name: 'Darth Vader',
},
};

const networkInterface = mockNetworkInterface(
{
request: { query: primeQuery },
result: { data: data1 },
},
{
request: { query: complexQuery },
result: { data: data2 },
}
);

const queryManager = new QueryManager({
networkInterface,
store: createApolloStore(),
reduxRootKey: 'apollo',
});

// First, prime the cache
queryManager.query({
query: primeQuery,
}).then(() => {
const handle = queryManager.watchQuery({
query: complexQuery,
noFetch: true,
});
let handleCount = 0;
handle.subscribe({
next(result) {
handleCount++;

if (handleCount === 1) {
assert.deepEqual(result.data, data1);
handle.refetch();
} else if (handleCount === 2) {
assert.deepEqual(result.data, data1);
}
},
});
});
});

it('runs a mutation', () => {
const mutation = gql`
mutation makeListPrivate {
Expand Down
1 change: 1 addition & 0 deletions test/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ describe('client', () => {
forceFetch: false,
fragmentMap: {},
returnPartialData: false,
noFetch: false,
lastRequestId: 1,
},
},
Expand Down
25 changes: 25 additions & 0 deletions test/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,31 @@ describe('reading from the store', () => {
}, /field missingField on object/);
});

it('does not throw on a missing field if noFetch is true', () => {
const result = {
id: 'abcd',
stringField: 'This is a string!',
numberField: 5,
nullField: null,
} as StoreObject;

const store = { abcd: result } as NormalizedCache;

assert.doesNotThrow(() => {
readFragmentFromStore({
store,
fragment: gql`
fragment FragmentName on Item {
stringField,
missingField
}
`,
rootId: 'abcd',
noFetch: true,
});
}, /field missingField on object/);
});

it('runs a nested fragment where the reference is null', () => {
const result = {
id: 'abcd',
Expand Down