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 query diffing entirely #693

Merged
merged 6 commits into from
Sep 30, 2016
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 3 to 6 months), to signal the start of a more stable API.

### vNEXT

- **Feature removal**: Remove query diffing functionality to make client more predictable and simplify implementation. Queries will still read from the store, and if the store does not have all of the necessary data the entire query will fetch from the server. Read justification and discussion in [Issue #615](https://github.com/apollostack/apollo-client/issues/615) [PR #693](https://github.com/apollostack/apollo-client/pull/693)

### v0.4.20
- Fix: Warn but do not fail when refetchQueries includes an unknown query name [PR #700](https://github.com/apollostack/apollo-client/pull/700)
- Fix: avoid field error on mutations after a query cancellation or a query failure by enforcing returnPartialData during previous data retrieval before applying a mutation update. [PR #696](https://github.com/apollostack/apollo-client/pull/696) and [Issue #647](https://github.com/apollostack/apollo-client/issues/647).
Expand Down
7 changes: 0 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,10 @@
"@types/promises-a-plus": "0.0.26",
"@types/redux": "^3.5.29",
"@types/sinon": "^1.16.29",
"async": "^2.0.0",
"browserify": "^13.0.0",
"chai": "^3.5.0",
"chai-as-promised": "^6.0.0",
"colors": "^1.1.2",
"concurrently": "^3.0.0",
"dataloader": "^1.1.0",
"es6-promise": "^3.1.2",
"typed-graphql": "1.0.2",
"grunt": "1.0.1",
Expand All @@ -88,14 +85,10 @@
"isomorphic-fetch": "^2.2.1",
"istanbul": "^0.4.5",
"lodash": "^4.6.1",
"lodash.mapvalues": "^4.3.0",
"lodash.omit": "^4.2.1",
"minimist": "^1.2.0",
"mocha": "^3.0.0",
"nodemon": "^1.9.2",
"pretty-bytes": "^4.0.0",
"remap-istanbul": "^0.5.1",
"request-promise": "^4.0.1",
"rxjs": "^5.0.0-beta.11",
"sinon": "^1.17.4",
"source-map-support": "^0.4.0",
Expand Down
101 changes: 16 additions & 85 deletions src/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import {
import {
GraphQLResult,
Document,
OperationDefinition,
FragmentDefinition,
// We need to import this here to allow TypeScript to include it in the definition file even
// though we don't use it. https://github.com/Microsoft/TypeScript/issues/5711
Expand All @@ -58,18 +57,13 @@ import {

import {
diffSelectionSetAgainstStore,
removeUnusedVariablesFromQuery,
} from './data/diffAgainstStore';

import {
MutationBehavior,
MutationQueryReducersMap,
} from './data/mutationResults';

import {
queryDocument,
} from './queryPrinting';

import {
QueryFetchRequest,
QueryBatcher,
Expand Down Expand Up @@ -179,7 +173,7 @@ export class QueryManager {
queryTransformer?: QueryTransformer,
resultTransformer?: ResultTransformer,
resultComparator?: ResultComparator,
shouldBatch?: Boolean,
shouldBatch?: boolean,
batchInterval?: number,
}) {
// XXX this might be the place to do introspection for inserting the `id` into the query? or
Expand Down Expand Up @@ -805,53 +799,6 @@ export class QueryManager {
};
}

// Takes a selection set for a query and diffs it against the store.
// Returns a query document of selection sets
// that must be fetched from the server and as well as the data returned from the store.
private handleDiffQuery({
queryDef,
rootId,
variables,
fragmentMap,
noFetch,
}: {
queryDef: OperationDefinition,
rootId: string,
variables: Object,
fragmentMap: FragmentMap,
noFetch: boolean,
}): {
diffedQuery: Document,
initialResult: Object,
} {
const { missingSelectionSets, result } = diffSelectionSetAgainstStore({
selectionSet: queryDef.selectionSet,
store: this.reduxRootSelector(this.store.getState()).data,
throwOnMissingField: false,
rootId,
variables,
fragmentMap,
});

const initialResult = result;
let diffedQuery: Document;
if (missingSelectionSets && missingSelectionSets.length && !noFetch) {
diffedQuery = queryDocument({
missingSelectionSets,
variableDefinitions: queryDef.variableDefinitions,
name: queryDef.name,
fragmentMap,
});

removeUnusedVariablesFromQuery(diffedQuery);
}

return {
diffedQuery,
initialResult,
};
}

// Takes a request id, query id, a query document and information associated with the query
// (e.g. variables, fragment map, etc.) and send it to the network interface. Returns
// a promise for the result associated with that request.
Expand Down Expand Up @@ -954,6 +901,7 @@ export class QueryManager {
queryDoc,
fragmentMap,
} = this.transformQueryDocument(options);

const queryDef = getQueryDefinition(queryDoc);
const queryString = print(queryDoc);
const querySS = {
Expand All @@ -962,52 +910,35 @@ export class QueryManager {
selectionSet: queryDef.selectionSet,
} as SelectionSetWithRoot;

// If we don't use diffing, then these will be the same as the original query, other than
// the queryTransformer that could have been applied.
let minimizedQueryString = queryString;
let minimizedQuery = querySS;
let minimizedQueryDoc = queryDoc;
let storeResult: any;
let needToFetch: boolean = forceFetch;

// If this is not a force fetch, we want to diff the query against the
// store before we fetch it from the network interface.
if (!forceFetch) {
const {
diffedQuery,
initialResult,
} = this.handleDiffQuery({
queryDef,
const { isMissing, result } = diffSelectionSetAgainstStore({
selectionSet: queryDef.selectionSet,
store: this.reduxRootSelector(this.store.getState()).data,
throwOnMissingField: false,
rootId: querySS.id,
variables,
fragmentMap,
noFetch,
});
storeResult = initialResult;
if (diffedQuery) {
minimizedQueryDoc = diffedQuery;
minimizedQueryString = print(minimizedQueryDoc);
minimizedQuery = {
id: querySS.id,
typeName: 'Query',
selectionSet: getQueryDefinition(diffedQuery).selectionSet,
} as SelectionSetWithRoot;
} else {
minimizedQueryDoc = null;
minimizedQueryString = null;
minimizedQuery = null;
}

// If we're in here, only fetch if we have missing fields
needToFetch = isMissing;

storeResult = result;
}

const requestId = this.generateRequestId();
const shouldFetch = minimizedQuery && !noFetch;
const shouldFetch = needToFetch && !noFetch;

// Initialize query in store with unique requestId
this.store.dispatch({
type: 'APOLLO_QUERY_INIT',
queryString,
query: querySS,
minimizedQueryString,
minimizedQuery,
variables,
forceFetch,
returnPartialData: returnPartialData || noFetch,
Expand All @@ -1027,7 +958,7 @@ export class QueryManager {
result: { data: storeResult },
variables,
query: querySS,
complete: !! minimizedQuery,
complete: !shouldFetch,
queryId,
});
}
Expand All @@ -1036,8 +967,8 @@ export class QueryManager {
return this.fetchRequest({
requestId,
queryId,
query: minimizedQueryDoc,
querySS: minimizedQuery,
query: queryDoc,
querySS,
options,
fragmentMap,
});
Expand Down
2 changes: 0 additions & 2 deletions src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ export interface QueryInitAction {
type: 'APOLLO_QUERY_INIT';
queryString: string;
query: SelectionSetWithRoot;
minimizedQueryString: string;
minimizedQuery: SelectionSetWithRoot;
variables: Object;
forceFetch: boolean;
returnPartialData: boolean;
Expand Down
4 changes: 2 additions & 2 deletions src/batching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class QueryBatcher {
// Queue on which the QueryBatcher will operate on a per-tick basis.
public queuedRequests: QueryFetchRequest[] = [];

private shouldBatch: Boolean;
private shouldBatch: boolean;
private pollInterval: Number;
private pollTimer: NodeJS.Timer | any; //oddity in Typescript

Expand All @@ -46,7 +46,7 @@ export class QueryBatcher {
shouldBatch,
networkInterface,
}: {
shouldBatch: Boolean,
shouldBatch: boolean,
networkInterface: NetworkInterface,
}) {
this.shouldBatch = shouldBatch;
Expand Down
Loading