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

Remove the query store from Redux #1859

Merged
merged 1 commit into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -1,6 +1,7 @@
# Change log

### vNEXT
- Remove query tracking from the Redux store. Query status tracking is now handled outside of Redux in the QueryStore class. [PR #1859](https://github.com/apollographql/apollo-client/pull/1859)
- Remove mutation tracking from the Redux store. Mutation status tracking is now handled outside of Redux in the MutationStore class. [PR #1846](https://github.com/apollographql/apollo-client/pull/1846)
- Use generic types for store updating functions in mutations [PR #1882](https://github.com/apollographql/apollo-client/pull/1882)
- Update to TypeScript 2.4.1 [PR #1892](https://github.com/apollographql/apollo-client/pull/1892)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"benchmark": "npm run compile:benchmark && node --stack-size=20000 lib/benchmark/index.js",
"benchmark:inspect": "npm run compile:benchmark && node --stack-size=20000 --inspect --debug-brk lib/benchmark/index.js",
"posttest": "npm run lint",
"filesize": "npm run compile:browser && ./scripts/filesize.js --file=./dist/index.min.js --maxGzip=27",
"filesize": "npm run compile:browser && ./scripts/filesize.js --file=./dist/index.min.js --maxGzip=28",
"flow-check": "flow check",
"compile": "tsc",
"compile:benchmark": "tsc -p tsconfig.test.json",
Expand Down
10 changes: 8 additions & 2 deletions src/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,10 @@ export default class ApolloClient implements DataProxy {
if (this.devToolsHookCb) {
this.devToolsHookCb({
action,
state: this.queryManager.getApolloState(),
state: {
queries: this.queryManager.queryStore.getStore(),
mutations: this.queryManager.mutationStore.getStore(),
},
dataWithOptimisticResults: this.queryManager.getDataWithOptimisticResults(),
});
}
Expand Down Expand Up @@ -485,7 +488,10 @@ export default class ApolloClient implements DataProxy {
if (this.devToolsHookCb) {
this.devToolsHookCb({
action,
state: this.queryManager.getApolloState(),
state: {
queries: this.queryManager.queryStore.getStore(),
mutations: this.queryManager.mutationStore.getStore(),
},
dataWithOptimisticResults: this.queryManager.getDataWithOptimisticResults(),
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
*/
public currentResult(): ApolloCurrentResult<T> {
const { data, partial } = this.queryManager.getCurrentQueryResult(this, true);
const queryStoreValue = this.queryManager.getApolloState().queries[this.queryId];
const queryStoreValue = this.queryManager.queryStore.get(this.queryId);

if (queryStoreValue && (
(queryStoreValue.graphQLErrors && queryStoreValue.graphQLErrors.length > 0) ||
Expand Down
123 changes: 88 additions & 35 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from './types';

import {
QueryStore,
QueryStoreValue,
} from '../queries/store';

Expand Down Expand Up @@ -137,12 +138,15 @@ import {
import { ObservableQuery } from './ObservableQuery';

export class QueryManager {
public static EMIT_REDUX_ACTIONS = true;

public pollingTimers: {[queryId: string]: any};
public scheduler: QueryScheduler;
public store: ApolloStore;
public networkInterface: NetworkInterface;
public ssrMode: boolean;
public mutationStore: MutationStore = new MutationStore();
public queryStore: QueryStore = new QueryStore();

private addTypename: boolean;
private deduplicator: Deduplicator;
Expand Down Expand Up @@ -182,6 +186,8 @@ export class QueryManager {

private lastRequestId: { [queryId: string]: number } = {};

private disableBroadcasting = false;

constructor({
networkInterface,
store,
Expand Down Expand Up @@ -298,7 +304,7 @@ export class QueryManager {
Object.keys(updateQueriesByName).forEach(queryName => (this.queryIdsByName[queryName] || []).forEach(queryId => {
ret[queryId] = {
reducer: updateQueriesByName[queryName],
query: this.getApolloState().queries[queryId],
query: this.queryStore.get(queryId),
};
}));
}
Expand Down Expand Up @@ -446,42 +452,64 @@ export class QueryManager {

// Initialize query in store with unique requestId
this.queryDocuments[queryId] = queryDoc;
this.store.dispatch({
type: 'APOLLO_QUERY_INIT',

this.queryStore.initQuery({
queryId,
queryString,
document: queryDoc,
operationName: getOperationName(queryDoc),
variables,
fetchPolicy,
queryId,
requestId,
// we store the old variables in order to trigger "loading new variables"
// state if we know we will go to the server
storePreviousVariables: shouldFetch,
variables,
isPoll: fetchType === FetchType.poll,
isRefetch: fetchType === FetchType.refetch,
fetchMoreForQueryId,
metadata,
fetchMoreForQueryId,
});

this.lastRequestId[queryId] = requestId;
this.broadcastQueries();

// 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.
const shouldDispatchClientResult = !shouldFetch || fetchPolicy === 'cache-and-network';
if (shouldDispatchClientResult) {
if (QueryManager.EMIT_REDUX_ACTIONS) {
this.store.dispatch({
type: 'APOLLO_QUERY_RESULT_CLIENT',
result: { data: storeResult },
variables,
type: 'APOLLO_QUERY_INIT',
queryString,
document: queryDoc,
operationName: getOperationName(queryDoc),
complete: !shouldFetch,
variables,
fetchPolicy,
queryId,
requestId,
// we store the old variables in order to trigger "loading new variables"
// state if we know we will go to the server
storePreviousVariables: shouldFetch,
isPoll: fetchType === FetchType.poll,
isRefetch: fetchType === FetchType.refetch,
fetchMoreForQueryId,
metadata,
});
}

this.lastRequestId[queryId] = requestId;

// 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.
const shouldDispatchClientResult = !shouldFetch || fetchPolicy === 'cache-and-network';
if (shouldDispatchClientResult) {
this.queryStore.markQueryResultClient(queryId, !shouldFetch);
this.broadcastQueries();

if (QueryManager.EMIT_REDUX_ACTIONS) {
this.store.dispatch({
type: 'APOLLO_QUERY_RESULT_CLIENT',
result: { data: storeResult },
variables,
document: queryDoc,
operationName: getOperationName(queryDoc),
complete: !shouldFetch,
queryId,
requestId,
});
}
}

if (shouldFetch) {
const networkResult = this.fetchRequest({
requestId,
Expand All @@ -496,13 +524,18 @@ export class QueryManager {
throw error;
} else {
if (requestId >= (this.lastRequestId[queryId] || 1)) {
this.store.dispatch({
type: 'APOLLO_QUERY_ERROR',
error,
queryId,
requestId,
fetchMoreForQueryId,
});
if (QueryManager.EMIT_REDUX_ACTIONS) {
this.store.dispatch({
type: 'APOLLO_QUERY_ERROR',
error,
queryId,
requestId,
fetchMoreForQueryId,
});
}

this.queryStore.markQueryError(queryId, error, fetchMoreForQueryId);
this.broadcastQueries();
}

this.removeFetchQueryPromise(requestId);
Expand Down Expand Up @@ -540,7 +573,7 @@ export class QueryManager {

// XXX This is to fix a strange race condition that was the root cause of react-apollo/#170
// queryStoreValue was sometimes the old queryStoreValue and not what's currently in the store.
queryStoreValue = this.getApolloState().queries[queryId];
queryStoreValue = this.queryStore.get(queryId);

const storedQuery = this.observableQueries[queryId];

Expand Down Expand Up @@ -766,10 +799,15 @@ export class QueryManager {
}

public stopQueryInStore(queryId: string) {
this.store.dispatch({
type: 'APOLLO_QUERY_STOP',
queryId,
});
this.queryStore.stopQuery(queryId);
this.broadcastQueries();

if (QueryManager.EMIT_REDUX_ACTIONS) {
this.store.dispatch({
type: 'APOLLO_QUERY_STOP',
queryId,
});
}
}

public getApolloState(): Store {
Expand Down Expand Up @@ -844,6 +882,8 @@ export class QueryManager {
reject(new Error('Store reset while query was in flight.'));
});

this.queryStore.reset(Object.keys(this.observableQueries));

this.store.dispatch({
type: 'APOLLO_STORE_RESET',
observableQueryIds: Object.keys(this.observableQueries),
Expand All @@ -859,7 +899,7 @@ export class QueryManager {
// store.
const observableQueryPromises: Promise<ApolloQueryResult<any>>[] = [];
Object.keys(this.observableQueries).forEach((queryId) => {
const storeQuery = this.reduxRootSelector(this.store.getState()).queries[queryId];
const storeQuery = this.queryStore.get(queryId);

const fetchPolicy = this.observableQueries[queryId].observableQuery.options.fetchPolicy;

Expand Down Expand Up @@ -1111,6 +1151,8 @@ export class QueryManager {

// default the lastRequestId to 1
if (requestId >= (this.lastRequestId[queryId] || 1)) {
this.disableBroadcasting = true;

// XXX handle multiple ApolloQueryResults
this.store.dispatch({
type: 'APOLLO_QUERY_RESULT',
Expand All @@ -1123,6 +1165,14 @@ export class QueryManager {
fetchMoreForQueryId,
extraReducers,
});

this.disableBroadcasting = false;

const { reducerError } = this.getApolloState();
if (!reducerError || reducerError.queryId !== queryId) {
this.queryStore.markQueryResult(queryId, result, fetchMoreForQueryId);
this.broadcastQueries();
}
}

this.removeFetchQueryPromise(requestId);
Expand Down Expand Up @@ -1191,7 +1241,10 @@ export class QueryManager {
}

private broadcastQueries() {
const queries = this.getApolloState().queries;
if (this.disableBroadcasting) {
return;
}

Object.keys(this.queryListeners).forEach((queryId: string) => {
const listeners = this.queryListeners[queryId];
// XXX due to an unknown race condition listeners can sometimes be undefined here.
Expand All @@ -1202,7 +1255,7 @@ export class QueryManager {
// it's possible for the listener to be undefined if the query is being stopped
// See here for more detail: https://github.com/apollostack/apollo-client/issues/231
if (listener) {
const queryStoreValue = queries[queryId];
const queryStoreValue = this.queryStore.get(queryId);
listener(queryStoreValue);
}
});
Expand Down
4 changes: 4 additions & 0 deletions src/mutations/store.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
export class MutationStore {
private store: {[mutationId: string]: MutationStoreValue} = {};

public getStore(): {[mutationId: string]: MutationStoreValue} {
return this.store;
}

public get(mutationId: string): MutationStoreValue {
return this.store[mutationId];
}
Expand Down
Loading