From 3030ec76651d31108547bffb2bf67290438a8a34 Mon Sep 17 00:00:00 2001 From: amandajliu Date: Tue, 5 Jul 2016 17:23:02 -0700 Subject: [PATCH 1/7] Moved methods, still debugging failed tests --- src/QueryManager.ts | 62 ++++++++++++++++------------------- src/scheduler.ts | 73 ++++++++++++++++++++++-------------------- src/util/Observable.ts | 14 +++++++- test/QueryManager.ts | 58 +++++++++++++++------------------ 4 files changed, 105 insertions(+), 102 deletions(-) diff --git a/src/QueryManager.ts b/src/QueryManager.ts index a676210f0b3..afab989deef 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -66,7 +66,7 @@ import { QueryScheduler, } from './scheduler'; -import { Observable, Observer, Subscription } from './util/Observable'; +import { Observable, Observer, Subscription, SubscriberFunction } from './util/Observable'; export class ObservableQuery extends Observable { public subscribe(observer: Observer): QuerySubscription { @@ -92,9 +92,6 @@ export class ObservableQuery extends Observable { } export interface QuerySubscription extends Subscription { - refetch(variables?: any): Promise; - stopPolling(): void; - startPolling(pollInterval: number): void; } export interface WatchQueryOptions { @@ -310,36 +307,12 @@ export class QueryManager { public watchQuery(options: WatchQueryOptions, shouldSubscribe = true): ObservableQuery { // Call just to get errors synchronously getQueryDefinition(options.query); + const queryId = this.generateQueryId(); const observableQuery = new ObservableQuery((observer) => { - const queryId = this.generateQueryId(); - const retQuerySubscription = { unsubscribe: () => { this.stopQuery(queryId); }, - refetch: (variables: any): Promise => { - // If no new variables passed, use existing variables - variables = variables || options.variables; - - // Use the same options as before, but with new variables and forceFetch true - return this.fetchQuery(queryId, assign(options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }, - stopPolling: (): void => { - if (this.pollingTimers[queryId]) { - clearInterval(this.pollingTimers[queryId]); - } - }, - startPolling: (pollInterval): void => { - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions); - }, pollInterval); - }, }; if (shouldSubscribe) { @@ -389,7 +362,31 @@ export class QueryManager { }); return retQuerySubscription; - }); + }, + (variables?: any) => { + // If no new variables passed, use existing variables + variables = variables || options.variables; + + // Use the same options as before, but with new variables and forceFetch true + return this.fetchQuery(queryId, assign(options, { + forceFetch: true, + variables, + }) as WatchQueryOptions); + }, + () => { + if (this.pollingTimers[queryId]) { + clearInterval(this.pollingTimers[queryId]); + } + }, + (pollInterval) => { + this.pollingTimers[queryId] = setInterval(() => { + const pollingOptions = assign({}, options) as WatchQueryOptions; + // subsequent fetches from polling always reqeust new data + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions); + }, pollInterval); + } + ); return observableQuery; } @@ -505,10 +502,7 @@ export class QueryManager { // the promise for it will be rejected and its results will not be written to the // store. Object.keys(this.observableQueries).forEach((queryId) => { - const subscriptions = this.observableQueries[queryId].subscriptions; - - // we can refetch any one of the subscriptions. - subscriptions[subscriptions.length - 1].refetch(); + this.observableQueries[queryId].observableQuery.refetch(); }); } diff --git a/src/scheduler.ts b/src/scheduler.ts index 4abdbb78cb1..8308a8fb239 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -100,47 +100,52 @@ export class QueryScheduler { if (!options.pollInterval) { throw new Error('Tried to register a non-polling query with the scheduler.'); } - - return new ObservableQuery((observer) => { - // "Fire" (i.e. add to the QueryBatcher queue) - const queryListener = this.queryManager.queryListenerForObserver(options, observer); - const queryId = this.startPollingQuery(options, queryListener); - - return { - unsubscribe: () => { - this.stopPollingQuery(queryId); - }, - - refetch: (variables: any): Promise => { - variables = variables || options.variables; - return this.fetchQuery(queryId, assign(options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }, - - startPolling: (pollInterval): void => { - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions).then(() => { - this.removeInFlight(queryId); - }); - }, pollInterval); - }, - - stopPolling: (): void => { - this.stopPollingQuery(queryId); - }, - }; - }); + const queryId = this.queryManager.generateQueryId(); + return new ObservableQuery( + (observer) => { + console.log('query id subscrip'); + console.log(queryId); + // "Fire" (i.e. add to the QueryBatcher queue) + const queryListener = this.queryManager.queryListenerForObserver(options, observer); + this.startPollingQuery(options, queryListener, queryId); + + return { + unsubscribe: () => { + this.stopPollingQuery(queryId); + }, + }; + }, + (variables: any) => { + console.log('query id refetch'); + console.log(queryId); + variables = variables || options.variables; + return this.fetchQuery(queryId, assign(options, { + forceFetch: true, + variables, + }) as WatchQueryOptions); + }, + () => { + this.pollingTimers[queryId] = setInterval(() => { + const pollingOptions = assign({}, options) as WatchQueryOptions; + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions).then(() => { + this.removeInFlight(queryId); + }); + }, options.pollInterval); + }, + () => { + this.stopPollingQuery(queryId); + } + ); } private addInFlight(queryId: string, options: WatchQueryOptions) { + console.log("adding in flight"); this.inFlightQueries[queryId] = options; } private removeInFlight(queryId: string) { + console.log("deleting in flight"); delete this.inFlightQueries[queryId]; } } diff --git a/src/util/Observable.ts b/src/util/Observable.ts index 8879b9ee3b8..3e3ee35b67d 100644 --- a/src/util/Observable.ts +++ b/src/util/Observable.ts @@ -2,6 +2,7 @@ // See https://github.com/zenparsing/es-observable import * as $$observable from 'symbol-observable'; +import { GraphQLResult } from 'graphql'; export type CleanupFunction = () => void; export type SubscriberFunction = (observer: Observer) => (Subscription | CleanupFunction); @@ -11,10 +12,19 @@ function isSubscription(subscription: Function | Subscription): subscription is } export class Observable { + public refetch: (variables?: any) => Promise; + public stopPolling: () => void; + public startPolling: (p: number) => void; private subscriberFunction: SubscriberFunction; - constructor(subscriberFunction: SubscriberFunction) { + constructor(subscriberFunction: SubscriberFunction, + refetch: (variables?: any) => Promise, + stopPolling: () => void, startPolling: (p: number) => void) { this.subscriberFunction = subscriberFunction; + this.refetch = refetch; + this.stopPolling = stopPolling; + this.startPolling = startPolling; + } public [$$observable]() { @@ -25,8 +35,10 @@ export class Observable { let subscriptionOrCleanupFunction = this.subscriberFunction(observer); if (isSubscription(subscriptionOrCleanupFunction)) { + console.log("is subscrip"); return subscriptionOrCleanupFunction; } else { + console.log("is cleanup"); return { unsubscribe: subscriptionOrCleanupFunction, }; diff --git a/test/QueryManager.ts b/test/QueryManager.ts index 7d572f8d3f2..9c726dfd2f9 100644 --- a/test/QueryManager.ts +++ b/test/QueryManager.ts @@ -1,8 +1,13 @@ import { QueryManager, QuerySubscription, + ObservableQuery, } from '../src/QueryManager'; +import { + Observer +} from '../src/util/Observable'; + import { createApolloStore, } from '../src/store'; @@ -561,13 +566,13 @@ describe('QueryManager', () => { variables, }); - const subscription = handle.subscribe({ + handle.subscribe({ next(result) { handleCount++; if (handleCount === 1) { assert.deepEqual(result.data, data1); - subscription.refetch(); + handle.refetch(); } else if (handleCount === 2) { assert.deepEqual(result.data, data2); done(); @@ -618,9 +623,9 @@ describe('QueryManager', () => { query, }); - const subscription = handle.subscribe({}); + handle.subscribe({}); - subscription.refetch().then((result) => { + handle.refetch().then((result) => { assert.deepEqual(result.data, data2); done(); }); @@ -684,16 +689,16 @@ describe('QueryManager', () => { query: query, }); - const subscription = handle.subscribe({ + handle.subscribe({ next(result) { handleCount++; if (handleCount === 1) { assert.deepEqual(result.data, data1); - subscription.refetch(); + handle.refetch(); } else if (handleCount === 2) { assert.deepEqual(result.data, data2); - subscription.refetch(variables); + handle.refetch(variables); } else if (handleCount === 3) { assert.deepEqual(result.data, data3); done(); @@ -757,18 +762,18 @@ describe('QueryManager', () => { let resultCount = 0; - const sub = handle.subscribe({ + handle.subscribe({ next(result) { resultCount++; // Perform refetch on first result from watchQuery if (resultCount === 1) { - sub.refetch(); + handle.refetch(); }; // Wait for a result count of 3 if (resultCount === 3) { // Stop polling - sub.stopPolling(); + handle.stopPolling(); assert(result); done(); } @@ -875,7 +880,7 @@ describe('QueryManager', () => { }); // Refetch before we get any data - maybe the network is slow, and the user clicked refresh? - subscription.refetch(); + handle.refetch(); }); }); @@ -1305,12 +1310,12 @@ describe('QueryManager', () => { }); let handleCount = 0; - const subscription = handle.subscribe({ + handle.subscribe({ next(result) { handleCount++; if (handleCount === 1) { assert.deepEqual(result.data, data1); - return subscription.refetch(); + return handle.refetch(); } else if (handleCount === 2) { assert.deepEqual(result.data, data2); store.dispatch({ @@ -1879,7 +1884,7 @@ describe('QueryManager', () => { }, }); - subscription.startPolling(50); + handle.startPolling(50); }); it('exposes a way to stop a polling query', (done) => { const query = gql` @@ -1931,12 +1936,12 @@ describe('QueryManager', () => { pollInterval: 50, }); - const subscription = handle.subscribe({ + handle.subscribe({ next(result) { handleCount++; if (handleCount === 2) { - subscription.stopPolling(); + handle.stopPolling(); } }, }); @@ -2275,33 +2280,20 @@ describe('QueryManager', () => { queryManager.resetStore(); }); - it('should call refetch on a mocked QuerySubscription if the store is reset', (done) => { - const mockQuerySubscription: QuerySubscription = { - unsubscribe() { - done(new Error('Unsubscribe was called on a subscription on store reset.')); - }, - + it('should call refetch on a mocked Observable if the store is reset', (done) => { + const mockObservableQuery: ObservableQuery = { refetch(variables: any): Promise { done(); return null; }, - - stopPolling(): void { - done(new Error('stopPolling was called on a subscription on store reset.')); - }, - - startPolling(pollInterval): void { - done(new Error('startPolling was called on a subscription on a store reset.')); - }, - }; + } as ObservableQuery; const queryManager = new QueryManager({ networkInterface: mockNetworkInterface(), store: createApolloStore(), reduxRootKey: 'apollo', }); const queryId = 'super-fake-id'; - queryManager.addObservableQuery(queryId, null); - queryManager.addQuerySubscription(queryId, mockQuerySubscription); + queryManager.addObservableQuery(queryId, mockObservableQuery); queryManager.resetStore(); }); From cc527e542e7b288ba7f6aa965f65a37bb2624d9f Mon Sep 17 00:00:00 2001 From: amandajliu Date: Wed, 6 Jul 2016 14:17:21 -0700 Subject: [PATCH 2/7] Fixed linter errors and test cases --- src/QueryManager.ts | 2 +- src/scheduler.ts | 14 ++------ src/util/Observable.ts | 2 -- test/QueryManager.ts | 72 ------------------------------------------ test/getFromAST.ts | 1 - 5 files changed, 3 insertions(+), 88 deletions(-) diff --git a/src/QueryManager.ts b/src/QueryManager.ts index afab989deef..c2632159059 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -66,7 +66,7 @@ import { QueryScheduler, } from './scheduler'; -import { Observable, Observer, Subscription, SubscriberFunction } from './util/Observable'; +import { Observable, Observer, Subscription } from './util/Observable'; export class ObservableQuery extends Observable { public subscribe(observer: Observer): QuerySubscription { diff --git a/src/scheduler.ts b/src/scheduler.ts index 8308a8fb239..dc6adc1f4c8 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -8,10 +8,6 @@ // At the moment, the QueryScheduler implements the one-polling-instance-at-a-time logic and // adds queries to the QueryBatcher queue. -import { - GraphQLResult, -} from 'graphql'; - import { ObservableQuery, WatchQueryOptions, @@ -63,9 +59,9 @@ export class QueryScheduler { queryId?: string): string { if (!queryId) { queryId = this.queryManager.generateQueryId(); - // Fire an initial fetch before we start the polling query - this.fetchQuery(queryId, options); } + // Fire an initial fetch before we start the polling query + this.fetchQuery(queryId, options); this.queryManager.addQueryListener(queryId, listener); this.pollingTimers[queryId] = setInterval(() => { @@ -103,8 +99,6 @@ export class QueryScheduler { const queryId = this.queryManager.generateQueryId(); return new ObservableQuery( (observer) => { - console.log('query id subscrip'); - console.log(queryId); // "Fire" (i.e. add to the QueryBatcher queue) const queryListener = this.queryManager.queryListenerForObserver(options, observer); this.startPollingQuery(options, queryListener, queryId); @@ -116,8 +110,6 @@ export class QueryScheduler { }; }, (variables: any) => { - console.log('query id refetch'); - console.log(queryId); variables = variables || options.variables; return this.fetchQuery(queryId, assign(options, { forceFetch: true, @@ -140,12 +132,10 @@ export class QueryScheduler { } private addInFlight(queryId: string, options: WatchQueryOptions) { - console.log("adding in flight"); this.inFlightQueries[queryId] = options; } private removeInFlight(queryId: string) { - console.log("deleting in flight"); delete this.inFlightQueries[queryId]; } } diff --git a/src/util/Observable.ts b/src/util/Observable.ts index 3e3ee35b67d..479ce03a36e 100644 --- a/src/util/Observable.ts +++ b/src/util/Observable.ts @@ -35,10 +35,8 @@ export class Observable { let subscriptionOrCleanupFunction = this.subscriberFunction(observer); if (isSubscription(subscriptionOrCleanupFunction)) { - console.log("is subscrip"); return subscriptionOrCleanupFunction; } else { - console.log("is cleanup"); return { unsubscribe: subscriptionOrCleanupFunction, }; diff --git a/test/QueryManager.ts b/test/QueryManager.ts index 9c726dfd2f9..2715609d37b 100644 --- a/test/QueryManager.ts +++ b/test/QueryManager.ts @@ -1,13 +1,8 @@ import { QueryManager, - QuerySubscription, ObservableQuery, } from '../src/QueryManager'; -import { - Observer -} from '../src/util/Observable'; - import { createApolloStore, } from '../src/store'; @@ -2335,73 +2330,6 @@ describe('QueryManager', () => { done(); }); }); - - it('should refetch the results even for observables with multiple subscriptions', (done) => { - let queryManager: QueryManager = null; - const query = gql` - query { - author { - firstName - lastName - } - }`; - const data = { - author: { - firstName: 'John', - lastName: 'Smith', - }, - }; - - let timesFired = 0; - let numResults = 0; - let numResultsSecond = 0; - - const myNetworkInterface: NetworkInterface = { - query(request: Request): Promise { - if (timesFired === 0) { - timesFired += 1; - queryManager.resetStore(); - } else { - timesFired += 1; - } - return Promise.resolve({ data }); - }, - }; - - queryManager = new QueryManager({ - networkInterface: myNetworkInterface, - store: createApolloStore(), - reduxRootKey: 'apollo', - }); - - const handle = queryManager.watchQuery({ query }); - - handle.subscribe({ - next(result) { - numResults += 1; - }, - - error(err) { - done(new Error('Errored on observable on store reset.')); - }, - }); - - handle.subscribe({ - next(result) { - numResultsSecond += 1; - }, - error(err) { - done(new Error('Errored on observable on store reset.')); - }, - }); - - setTimeout(() => { - assert.equal(timesFired, 3); - assert.equal(numResults, 2); - assert.equal(numResultsSecond, 1); - done(); - }, 100); - }); }); describe('fragment referencing', () => { diff --git a/test/getFromAST.ts b/test/getFromAST.ts index 55830f3b486..5962c9accc1 100644 --- a/test/getFromAST.ts +++ b/test/getFromAST.ts @@ -99,7 +99,6 @@ describe('AST utility functions', () => { expectedDoc.definitions[0] as FragmentDefinition, expectedDoc.definitions[1] as FragmentDefinition, ]; - const actualResult = getFragmentDefinitions(multipleFragmentDefinitions); assert.deepEqual(actualResult.map(print), expectedResult.map(print)); }); From 78540ae58ed76b4c3061d9e56a162e1ad07aedfb Mon Sep 17 00:00:00 2001 From: amandajliu Date: Wed, 6 Jul 2016 14:29:02 -0700 Subject: [PATCH 3/7] Updated change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efe42defbaf..5cebe3cfc22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ 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 +- moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery [Issue #194] [PR #362] ### v0.3.30 From 5fc2545f026e15de24c9b3d13e34c685140d1c77 Mon Sep 17 00:00:00 2001 From: amandajliu Date: Thu, 7 Jul 2016 11:38:44 -0700 Subject: [PATCH 4/7] cleaned up code --- CHANGELOG.md | 2 +- src/QueryManager.ts | 80 +++++++++++++++++++++++++----------------- src/scheduler.ts | 45 ++++++++++++++---------- src/util/Observable.ts | 11 +----- 4 files changed, 76 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cebe3cfc22..71bd4f6fcce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ 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 -- moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery [Issue #194] [PR #362] +- moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery [Issue #194] (https://github.com/apollostack/apollo-client/issues/194) and [PR #362] (https://github.com/apollostack/apollo-client/pull/362) ### v0.3.30 diff --git a/src/QueryManager.ts b/src/QueryManager.ts index c2632159059..d155c57f1cc 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -66,11 +66,24 @@ import { QueryScheduler, } from './scheduler'; -import { Observable, Observer, Subscription } from './util/Observable'; +import { Observable, Observer, Subscription, SubscriberFunction } from './util/Observable'; export class ObservableQuery extends Observable { - public subscribe(observer: Observer): QuerySubscription { - return super.subscribe(observer) as QuerySubscription; + public refetch: (variables?: any) => Promise; + public stopPolling: () => void; + public startPolling: (p: number) => void; + + constructor(subscriberFunction: SubscriberFunction, + refetch: (variables?: any) => Promise, + stopPolling: () => void, startPolling: (p: number) => void) { + super(subscriberFunction); + this.refetch = refetch; + this.stopPolling = stopPolling; + this.startPolling = startPolling; + + } + public subscribe(observer: Observer): Subscription { + return super.subscribe(observer) as Subscription; } @@ -91,9 +104,6 @@ export class ObservableQuery extends Observable { } } -export interface QuerySubscription extends Subscription { -} - export interface WatchQueryOptions { query: Document; variables?: { [key: string]: any }; @@ -134,7 +144,7 @@ export class QueryManager { // with them in case of some destabalizing action (e.g. reset of the Apollo store). private observableQueries: { [queryId: string]: { observableQuery: ObservableQuery; - subscriptions: QuerySubscription[]; + subscriptions: Subscription[]; } }; constructor({ @@ -308,6 +318,33 @@ export class QueryManager { // Call just to get errors synchronously getQueryDefinition(options.query); const queryId = this.generateQueryId(); + + const refetch = (variables?: any) => { + // If no new variables passed, use existing variables + variables = variables || options.variables; + + // Use the same options as before, but with new variables and forceFetch true + return this.fetchQuery(queryId, assign(options, { + forceFetch: true, + variables, + }) as WatchQueryOptions); + }; + + const stopPolling = () => { + if (this.pollingTimers[queryId]) { + clearInterval(this.pollingTimers[queryId]); + } + }; + + const startPolling = (pollInterval) => { + this.pollingTimers[queryId] = setInterval(() => { + const pollingOptions = assign({}, options) as WatchQueryOptions; + // subsequent fetches from polling always reqeust new data + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions); + }, pollInterval); + }; + const observableQuery = new ObservableQuery((observer) => { const retQuerySubscription = { unsubscribe: () => { @@ -360,32 +397,11 @@ export class QueryManager { } } }); - return retQuerySubscription; }, - (variables?: any) => { - // If no new variables passed, use existing variables - variables = variables || options.variables; - - // Use the same options as before, but with new variables and forceFetch true - return this.fetchQuery(queryId, assign(options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }, - () => { - if (this.pollingTimers[queryId]) { - clearInterval(this.pollingTimers[queryId]); - } - }, - (pollInterval) => { - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions); - }, pollInterval); - } + refetch, + stopPolling, + startPolling ); return observableQuery; @@ -463,7 +479,7 @@ export class QueryManager { } // Associates a query subscription with an ObservableQuery in this.observableQueries - public addQuerySubscription(queryId: string, querySubscription: QuerySubscription) { + public addQuerySubscription(queryId: string, querySubscription: Subscription) { if (this.observableQueries.hasOwnProperty(queryId)) { this.observableQueries[queryId].subscriptions.push(querySubscription); } else { diff --git a/src/scheduler.ts b/src/scheduler.ts index dc6adc1f4c8..490ee91b242 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -97,6 +97,29 @@ export class QueryScheduler { throw new Error('Tried to register a non-polling query with the scheduler.'); } const queryId = this.queryManager.generateQueryId(); + + const refetch = (variables: any) => { + variables = variables || options.variables; + return this.fetchQuery(queryId, assign(options, { + forceFetch: true, + variables, + }) as WatchQueryOptions); + }; + + const startPolling = () => { + this.pollingTimers[queryId] = setInterval(() => { + const pollingOptions = assign({}, options) as WatchQueryOptions; + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions).then(() => { + this.removeInFlight(queryId); + }); + }, options.pollInterval); + }; + + const stopPolling = () => { + this.stopPollingQuery(queryId); + }; + return new ObservableQuery( (observer) => { // "Fire" (i.e. add to the QueryBatcher queue) @@ -109,25 +132,9 @@ export class QueryScheduler { }, }; }, - (variables: any) => { - variables = variables || options.variables; - return this.fetchQuery(queryId, assign(options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }, - () => { - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions).then(() => { - this.removeInFlight(queryId); - }); - }, options.pollInterval); - }, - () => { - this.stopPollingQuery(queryId); - } + refetch, + stopPolling, + startPolling ); } diff --git a/src/util/Observable.ts b/src/util/Observable.ts index 479ce03a36e..51c56f88f78 100644 --- a/src/util/Observable.ts +++ b/src/util/Observable.ts @@ -2,7 +2,6 @@ // See https://github.com/zenparsing/es-observable import * as $$observable from 'symbol-observable'; -import { GraphQLResult } from 'graphql'; export type CleanupFunction = () => void; export type SubscriberFunction = (observer: Observer) => (Subscription | CleanupFunction); @@ -12,18 +11,10 @@ function isSubscription(subscription: Function | Subscription): subscription is } export class Observable { - public refetch: (variables?: any) => Promise; - public stopPolling: () => void; - public startPolling: (p: number) => void; private subscriberFunction: SubscriberFunction; - constructor(subscriberFunction: SubscriberFunction, - refetch: (variables?: any) => Promise, - stopPolling: () => void, startPolling: (p: number) => void) { + constructor(subscriberFunction: SubscriberFunction) { this.subscriberFunction = subscriberFunction; - this.refetch = refetch; - this.stopPolling = stopPolling; - this.startPolling = startPolling; } From 6fb9a7a57a4536c2811c38890a0669d6e30d2580 Mon Sep 17 00:00:00 2001 From: Sashko Stubailo Date: Mon, 11 Jul 2016 12:27:39 -0700 Subject: [PATCH 5/7] Clean up observablequery class --- CHANGELOG.md | 1 + src/QueryManager.ts | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71bd4f6fcce..c53a0455eb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ 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 + - moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery [Issue #194] (https://github.com/apollostack/apollo-client/issues/194) and [PR #362] (https://github.com/apollostack/apollo-client/pull/362) ### v0.3.30 diff --git a/src/QueryManager.ts b/src/QueryManager.ts index d155c57f1cc..905b3ce6b76 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -73,20 +73,23 @@ export class ObservableQuery extends Observable { public stopPolling: () => void; public startPolling: (p: number) => void; - constructor(subscriberFunction: SubscriberFunction, + constructor( + subscriberFunction: SubscriberFunction, refetch: (variables?: any) => Promise, - stopPolling: () => void, startPolling: (p: number) => void) { - super(subscriberFunction); - this.refetch = refetch; - this.stopPolling = stopPolling; - this.startPolling = startPolling; + stopPolling: () => void, + startPolling: (p: number) => void + ) { + super(subscriberFunction); + this.refetch = refetch; + this.stopPolling = stopPolling; + this.startPolling = startPolling; } + public subscribe(observer: Observer): Subscription { - return super.subscribe(observer) as Subscription; + return super.subscribe(observer); } - public result(): Promise { return new Promise((resolve, reject) => { const subscription = this.subscribe({ From 901c7f435172de0e1da77304f267c5f9d80686e8 Mon Sep 17 00:00:00 2001 From: Sashko Stubailo Date: Mon, 11 Jul 2016 12:35:25 -0700 Subject: [PATCH 6/7] Refactor to use keyword arguments --- src/QueryManager.ts | 80 ++++++++++++++++++++++++--------------------- src/scheduler.ts | 30 +++++++++-------- 2 files changed, 58 insertions(+), 52 deletions(-) diff --git a/src/QueryManager.ts b/src/QueryManager.ts index 905b3ce6b76..5667d9465a6 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -73,17 +73,16 @@ export class ObservableQuery extends Observable { public stopPolling: () => void; public startPolling: (p: number) => void; - constructor( + constructor(options: { subscriberFunction: SubscriberFunction, refetch: (variables?: any) => Promise, stopPolling: () => void, startPolling: (p: number) => void - ) { - super(subscriberFunction); - this.refetch = refetch; - this.stopPolling = stopPolling; - this.startPolling = startPolling; - + }) { + super(options.subscriberFunction); + this.refetch = options.refetch; + this.stopPolling = options.stopPolling; + this.startPolling = options.startPolling; } public subscribe(observer: Observer): Subscription { @@ -322,33 +321,9 @@ export class QueryManager { getQueryDefinition(options.query); const queryId = this.generateQueryId(); - const refetch = (variables?: any) => { - // If no new variables passed, use existing variables - variables = variables || options.variables; - - // Use the same options as before, but with new variables and forceFetch true - return this.fetchQuery(queryId, assign(options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }; - - const stopPolling = () => { - if (this.pollingTimers[queryId]) { - clearInterval(this.pollingTimers[queryId]); - } - }; - - const startPolling = (pollInterval) => { - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions); - }, pollInterval); - }; + let observableQuery; - const observableQuery = new ObservableQuery((observer) => { + const subscriberFunction = (observer) => { const retQuerySubscription = { unsubscribe: () => { this.stopQuery(queryId); @@ -401,11 +376,40 @@ export class QueryManager { } }); return retQuerySubscription; - }, - refetch, - stopPolling, - startPolling - ); + }; + + const refetch = (variables?: any) => { + // If no new variables passed, use existing variables + variables = variables || options.variables; + + // Use the same options as before, but with new variables and forceFetch true + return this.fetchQuery(queryId, assign(options, { + forceFetch: true, + variables, + }) as WatchQueryOptions); + }; + + const stopPolling = () => { + if (this.pollingTimers[queryId]) { + clearInterval(this.pollingTimers[queryId]); + } + }; + + const startPolling = (pollInterval) => { + this.pollingTimers[queryId] = setInterval(() => { + const pollingOptions = assign({}, options) as WatchQueryOptions; + // subsequent fetches from polling always reqeust new data + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions); + }, pollInterval); + }; + + observableQuery = new ObservableQuery({ + subscriberFunction, + refetch, + stopPolling, + startPolling, + }); return observableQuery; } diff --git a/src/scheduler.ts b/src/scheduler.ts index 490ee91b242..f6671f75362 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -98,6 +98,18 @@ export class QueryScheduler { } const queryId = this.queryManager.generateQueryId(); + const subscriberFunction = (observer) => { + // "Fire" (i.e. add to the QueryBatcher queue) + const queryListener = this.queryManager.queryListenerForObserver(options, observer); + this.startPollingQuery(options, queryListener, queryId); + + return { + unsubscribe: () => { + this.stopPollingQuery(queryId); + }, + }; + }; + const refetch = (variables: any) => { variables = variables || options.variables; return this.fetchQuery(queryId, assign(options, { @@ -120,22 +132,12 @@ export class QueryScheduler { this.stopPollingQuery(queryId); }; - return new ObservableQuery( - (observer) => { - // "Fire" (i.e. add to the QueryBatcher queue) - const queryListener = this.queryManager.queryListenerForObserver(options, observer); - this.startPollingQuery(options, queryListener, queryId); - - return { - unsubscribe: () => { - this.stopPollingQuery(queryId); - }, - }; - }, + return new ObservableQuery({ + subscriberFunction, refetch, stopPolling, - startPolling - ); + startPolling, + }); } private addInFlight(queryId: string, options: WatchQueryOptions) { From 63683073957a5cbb32a070a44c7cc563cc2eb6d3 Mon Sep 17 00:00:00 2001 From: Sashko Stubailo Date: Mon, 11 Jul 2016 12:37:46 -0700 Subject: [PATCH 7/7] Make changelog more detailed --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c53a0455eb1..e9c897a477d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Expect active development and potentially significant breaking changes in the `0 ### vNEXT -- moved refetch(), startPolling(), and stopPolling() methods from QuerySubscription to ObservableQuery [Issue #194] (https://github.com/apollostack/apollo-client/issues/194) and [PR #362] (https://github.com/apollostack/apollo-client/pull/362) +- **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) ### v0.3.30