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

Move refetch(), startPolling() and stopPolling() from Subscription to ObservableQuery #194

Closed
martijnwalraven opened this issue May 10, 2016 · 5 comments
Assignees

Comments

@martijnwalraven
Copy link
Contributor

martijnwalraven commented May 10, 2016

Interoperability with other Observable implementations (see #149) means we have no control over the Subscription object users end up with.

For example, Rx.Observable.from(handle).subscribe() would return an RxJS Subscription object without the Apollo-specific helper methods.

So I think it makes most sense to move these methods to ObservableQuery (maybe this needs a better name, like QueryHandle or simply Query).

This will require some refactoring of the current code to share fetching and polling logic between observers, but that actually seems like a better design anyway.

const handle = queryManager.watchQuery({ query });
handle.refetch();
handle.startPolling();
handle.stopPolling();
@martijnwalraven martijnwalraven self-assigned this May 10, 2016
@jbaxleyiii
Copy link
Contributor

@martijnwalraven I'm all for it!

@stubailo
Copy link
Contributor

@Urigo, @kamilkisiela I think we should fix this ASAP. I was just talking to @purban and it's a bit weird because people from Angular 2 are going to be expecting RxJS, not a regular old observable. With this merged, at least we can implement a feature to replace the observable shim with Rx.

stubailo pushed a commit that referenced this issue Jul 11, 2016
@zol zol removed the ready label Jul 13, 2016
@kamilkisiela
Copy link
Contributor

kamilkisiela commented Jul 20, 2016

Maybe you guys will be interested in it.

Here's an implementation of QueryObservable that works with RxJS.


import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/from';

const origin = client.watchQuery(options); // QueryObservable
const obs = Observable.from(origin);

origin.refetch(); // works
obs.refetch(); // refetch is undefined
obs.ish.refetch(); // works (it should) because `ish` is a reference to `origin`

To solve that problem I created a custom observable called ApolloQueryObservable that extends Observable from RxJS. QueryObservable is being hold under apollo property. This way I could create methods like refetch(), startPolling(), stopPolling().


import 'rxjs';
import { ApolloQueryObservable } from 'angular2-apollo';

const origin = client.watchQuery(options); // QueryObservable
const obs = new ApolloQueryObservable(origin);

origin.refetch(); // works
obs.refetch(); // works

obs.map(customMapFunction).refetch() // refetch is undefined

As you can see, it still won't work with operators. Here's why.

Every operator uses Observable.lift() method, for example map().

Observable.lift() creates new Observable, then assigns the current observable under source property. Here the moment where our custom methods (refetch etc) are being dropped.

I solved this by overwriting lift() method.


@stubailo @martijnwalraven @jbaxleyiii @Urigo I was thinking about moving this ApolloQueryObservable (in different name, whatever) to ApolloClient, because it's not angular2 specific and will help a lot of people, I think.

@stubailo
Copy link
Contributor

Hmm, sounds like this could be a good idea. Could you open a new issue with some details about how ApolloQueryObservable works?

@kamilkisiela
Copy link
Contributor

#430

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants