Merge Observer and Subscription Types?

[benlesh]

Based on discussions in #69, #70 and (mostly) #71.

Should the interfaces of Observer and Subscription be joined into a single type? Right now, generally, Subscriptions are created 1:1 with (safe) Observers. They tend to have to "know" about each other or at least notify one another to update state such as isUnsubscribed. And there might be some wins to joining them into a single class.

This would also definitely affect the lift method, and ObserverFactories would have to become SubscriptionFactories or the like, I suppose.

Pros? Cons? Good idea? Horrible idea?

[headinthebox]

Why not. Historically they came from IDisposable, which was used by itself, but that is not the case here.

[benlesh]

Based on the parts of the discussion in #71, I think this is a solid idea. Can I get more opinions, @trxcllnt? @vsavkin? @staltz?

The proposal is this... the two interfaces:

interface Observer {
  next(value: any): void
  error(err: any): void
  complete(value: any): void
  subscribe(subscription: Subscription)
}

interface Subscription {
  unsubscribe(): void
  isUnsubscribed: boolean
  add(subscription: Subscription)
  remove(subscription: Subscription)
}

be implemented/combined on a single class:

interface Subscriber {
  next(value: any): void
  error(err: any): void
  complete(value: any): void
  subscribe()
  unsubscribe(): void
  isUnsubscribed: boolean
  add(subscription: Subscription)
  remove(subscription: Subscription)
}

Notice the subscribe event has changed, since it doesn't need to pass itself to itself.

[benlesh]

I'd also love @zenparsing's input, since they are bike shedding the related things in the es-observable spec.

[headinthebox]

+1, but call it Observer.

[ktrott]

RxJS Next meeting notes
(attendees: @Blesh, @jhusain, @trxcllnt, @benjchristensen, @steveorsomethin)
Agreed to move forward with @Blesh proposal

[staltz]

How would this single interface work to support the use case of Observers in the lift operator?

interface Subscriber {
  next(value: any): void // <---- notice the void return
  error(err: any): void
  complete(value: any): void
  subscribe()
  unsubscribe(): void
  isUnsubscribed: boolean
  add(subscription: Subscription)
  remove(subscription: Subscription)
}

The map example from issue #60

class Observable {
  constructor(subscribe) {
    if(subscribe) {
      this.subscribe = subscribe;
    }
  }
  subscribe(observer) {
    return this.source.subscribe(this.observerFactory.create(observer));
  }
  lift(observerFactory) {
    const o = new Observable();
    o.source = this;
    o.observerFactory = observerFactory;
    return o;
  }
  map(selector) {
    return this.lift(new MapObserverFactory(selector));
  }
}

class MapObserverFactory {
  constructor(selector) {
    this.selector = selector;
  }
  create(destination) {
    return new MapObserver(destination, this.selector);
  }
}

class MapObserver {
  constructor(destination, selector) {
    this.selector = selector;
    this.destination = destination;
  }
  next(x) {
    return this.destination.next(this.selector(x)); // <--- notice, not a void return
  }
  throw(e) {
    return this.destination.throw(e);
  }
  return(e) {
    return this.destination.return(e);
  }
}

[staltz]

To clarify a bit: I find it weird when I see an Observer returning a value in next(). An Observer is a consumer/sink, so it shouldn't need to return anything. In RxJava, lift doesn't get an Observer, it gets an Operator, which implements Subscriber and Observer together with Func1.

[benlesh]

How would this single interface work to support the use case of Observers in the lift operator?

Well, it's it's not really a single interface, it's a single class that implements two interfaces. A Subscriber could be used as an Observer or could be used as a Subscription. The lift implementation shouldn't change much if at all.

I find it weird when I see an Observer returning a value in next()

@staltz ... void return is a void return. It's the result of an absence of a return. Just TypeScript semantics. FWIW: the ES7 Observable spec is bike shedding an interface here that shows next returning any... like you, I'm not sure how that is of value. So it's probably worth adding your voice over there.

[staltz]

void return is a void return. It's the result of an absence of a return. Just TypeScript semantics.

@Blesh I know 🌝

[benlesh]

Done in master now.