[WIP] Error handling and Observable support

[martijnwalraven]

To continue the discussion in #88:

I'm not sure I like having a separate onError callback. It also forces us to keep maps of queryId to both result and error callbacks, and possibly more. The design that makes the most sense to me is something like:

export interface QueryObserver {
  onResult: (result: GraphQLResult) => void;
  onError: (error: Error) => void;
  onStop?: () => void;
}

handle.subscribe({
  onResult: (result) => {
  },
  onError: (error) => {
  },
});

We could also allow passing in an observer to watchQuery() for convenience:

const handle = queryManager.watchQuery({
  query,
  {
    onResult: (result) => {
    },
    onError: (error) => {
    },
  }
});

This looks a lot like Observable<GraphQLResult>, apart from the callback names. So we could decide to make QueryObserver compatible.

(I've kept handle.onResult() for now to avoid rewriting all test code, but the idea is that we would remove it.)

[stubailo]

Hmm, I think that's pretty reasonable - and then we'll keep track of these observer objects instead of individual callbacks?

For what it's worth, I think watchQuery would be better like:

const handle = queryManager.watchQuery({
  query,
  onResult: (result) => { },
  onError: (error) => { },
});

Tell me more - what would an observable for this look like? Seems like it's a pretty similar interface so if it allows us to be more standardized that could be a big win.

[martijnwalraven]

Yes, in the code in this pull request we store an array of observers per queryId. Alternatively, we could store something like a QueryHandle object by queryId and use that to store observers, the last result/error, etc.

QueryObserver is basically the same as an Observer<GraphQLResult>, apart from the callback names. The QueryHandle could implement Observable<GraphQLResult> and would offer a subscribe() method:

let subscription = handle.subscribe({
  next(result: GraphQLResult) { },
  error(error) { },
  complete() { }
});

The Subscription returned could be used to unsubscribe:

subscription.unsubscribe();

This would only unsubscribe one observer, so the query should only be stopped when there are no subscribers left.

Hmmm, that means passing in an observer to watchQuery() could be problematic, because there is no way to return the subscription in that case (just the handle, which is shared between subscriptions).

Not all of Observable has been properly standardized yet, but the idea is that you can use methods like filter() and map() on every observable.

[martijnwalraven]

This is still very much work in progress, but I think we're getting somewhere. I'm in the midst of some refactoring and there are some timing issues to fix, so don't mind the code so much, but apart from the naming this is the API as it is working now:

const observable = queryManager.observeQuery({ query, variables, forceFetch, returnPartialData });

// Query isn't started until the first observer subscribes (part of the Observable contract)

const subscription = observable.subscribe({
  next(result) {
    // Can be invoked synchronously when the result is cached
  },
  error(error) {
  },
});

// If the last observer unsubscribes, the query is no longer observed (and there are no references kept, so it may be garbage collected)

subscription.unsubscribe();

// Alternatively, returning a promise
observable.fetch().then(result => {
});

@Urigo: Does this look good to you for the Angular integration?

[stubailo]

Wow this looks sweet!

[jbaxleyiii]

@martijnwalraven this looks really killer!

[stubailo]

I merged in the network mock refactor and the basic error handling, and reconstructed this PR on top of those two in a new PR: #119

Going to close this one in favor of that one.