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

refactor data reducer #383

Closed
wants to merge 1 commit into from
Closed

refactor data reducer #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/QueryManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ export class QueryManager {
variables,
mutationId,
fragmentMap: queryFragmentMap,
mutations: this.getApolloState().mutations,
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to pass in all of the state about mutations and queries into the reducer - we only need the particular query this result is associated with. Same goes for the rest.

Copy link
Contributor Author

@abhiaiyer91 abhiaiyer91 Jul 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

okay so you wanna move that functionality out of data as well? so instead of returning mutationId or queryId in actions, we just cruise with the actual mutation/query we're dealing with?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still need mutationId in the store to keep track of loading/error state.

I think this change is great, we just should only pass in the data we actually need, which is basically only the single mutation, not all mutations ever sent.

});

return this.networkInterface.query(request)
Expand All @@ -258,6 +259,7 @@ export class QueryManager {
result,
mutationId,
resultBehaviors,
mutations: this.getApolloState().mutations,
});

return result;
Expand Down Expand Up @@ -453,6 +455,7 @@ export class QueryManager {
this.store.dispatch({
type: 'APOLLO_QUERY_STOP',
queryId,
queries: this.getApolloState().queries,
});
};

Expand Down Expand Up @@ -624,6 +627,7 @@ export class QueryManager {
queryId,
requestId,
fragmentMap: queryFragmentMap,
queries: this.getApolloState().queries,
});

if (! minimizedQuery || returnPartialData) {
Expand All @@ -636,6 +640,7 @@ export class QueryManager {
query: querySS,
complete: !! minimizedQuery,
queryId,
queries: this.getApolloState().queries
});
}

Expand Down Expand Up @@ -663,6 +668,7 @@ export class QueryManager {
result,
queryId,
requestId,
queries: this.getApolloState().queries
});

this.removeFetchQueryPromise(requestId);
Expand Down Expand Up @@ -696,6 +702,7 @@ export class QueryManager {
error,
queryId,
requestId,
queries: this.getApolloState().queries,
});

this.removeFetchQueryPromise(requestId);
Expand Down Expand Up @@ -740,6 +747,7 @@ export class QueryManager {
this.store.dispatch({
type: 'APOLLO_QUERY_STOP',
queryId,
queries: this.getApolloState().queries,
});
}

Expand Down
12 changes: 12 additions & 0 deletions src/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ import {

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

import {
MutationStore,
} from './mutations/store';

import {
MutationBehavior,
} from './data/mutationResults';
Expand All @@ -15,6 +20,7 @@ import { FragmentMap } from './queries/getFromAST';
export interface QueryResultAction {
type: 'APOLLO_QUERY_RESULT';
result: GraphQLResult;
queries: QueryStore;
queryId: string;
requestId: number;
}
Expand All @@ -27,6 +33,7 @@ export interface QueryErrorAction {
type: 'APOLLO_QUERY_ERROR';
error: Error;
queryId: string;
queries: QueryStore;
requestId: number;
}

Expand All @@ -45,6 +52,7 @@ export interface QueryInitAction {
returnPartialData: boolean;
queryId: string;
requestId: number;
queries: QueryStore;
fragmentMap: FragmentMap;
}

Expand All @@ -56,6 +64,7 @@ export interface QueryResultClientAction {
type: 'APOLLO_QUERY_RESULT_CLIENT';
result: GraphQLResult;
complete: boolean;
queries: QueryStore;
queryId: string;
}

Expand All @@ -66,6 +75,7 @@ export function isQueryResultClientAction(action: ApolloAction): action is Query
export interface QueryStopAction {
type: 'APOLLO_QUERY_STOP';
queryId: string;
queries: QueryStore;
}

export function isQueryStopAction(action: ApolloAction): action is QueryStopAction {
Expand All @@ -79,6 +89,7 @@ export interface MutationInitAction {
variables: Object;
mutationId: string;
fragmentMap: FragmentMap;
mutations: MutationStore;
}

export function isMutationInitAction(action: ApolloAction): action is MutationInitAction {
Expand All @@ -90,6 +101,7 @@ export interface MutationResultAction {
result: GraphQLResult;
mutationId: string;
resultBehaviors?: MutationBehavior[];
mutations: MutationStore;
}

export function isMutationResultAction(action: ApolloAction): action is MutationResultAction {
Expand Down
19 changes: 5 additions & 14 deletions src/data/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ import {

import assign = require('lodash.assign');

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

import {
MutationStore,
} from '../mutations/store';

import {
ApolloReducerConfig,
} from '../store';
Expand Down Expand Up @@ -46,25 +38,24 @@ export type StoreValue = number | string | string[];
export function data(
previousState: NormalizedCache = {},
action: ApolloAction,
queries: QueryStore,
mutations: MutationStore,
config: ApolloReducerConfig
): NormalizedCache {

if (isQueryResultAction(action)) {
if (!queries[action.queryId]) {
if (!action.queries[action.queryId]) {
return previousState;
}

// Ignore results from old requests
// XXX this means that if you have a refetch interval which is shorter than your roundtrip time,
// your query will be in the loading state forever!
if (action.requestId < queries[action.queryId].lastRequestId) {
if (action.requestId < action.queries[action.queryId].lastRequestId) {
return previousState;
}

// XXX handle partial result due to errors
if (! graphQLResultHasError(action.result)) {
const queryStoreValue = queries[action.queryId];
const queryStoreValue = action.queries[action.queryId];

// XXX use immutablejs instead of cloning
const clonedState = assign({}, previousState) as NormalizedCache;
Expand All @@ -84,7 +75,7 @@ export function data(
} else if (isMutationResultAction(action)) {
// Incorporate the result from this mutation into the store
if (!action.result.errors) {
const queryStoreValue = mutations[action.mutationId];
const queryStoreValue = action.mutations[action.mutationId];

// XXX use immutablejs instead of cloning
const clonedState = assign({}, previousState) as NormalizedCache;
Expand Down
2 changes: 1 addition & 1 deletion src/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function createApolloReducer(config: ApolloReducerConfig): Function {

// Note that we are passing the queries into this, because it reads them to associate
// the query ID in the result with the actual query
data: data(state.data, action, state.queries, state.mutations, config),
data: data(state.data, action, config),
};

return newState;
Expand Down