fix(Observer): observer interface now allows partial Observers

[benlesh]

• caveat is that there is no way to require at least one method on Observer in TypeScript, at least that I can find

related #1260

[kwonoj]

Looks good to me for now. I'll track how typescript side changes / updates are going to followup further.

[kwonoj]

Seems with current Typescript it can be achieved via below, not looking elegant bit though..

type Observer<T> = NextObserver<T> | ErrorObserver | CompleteObserver | (NextObserver<T> & ErrorObserver & CompleteObserver) | (ErrorObserver & CompleteObserver) | (NextObserver<T> & ErrorObserver) | (NextObserver<T> & CompleteObserver);

(copy-pasted from issue).

[benlesh]

I experimented for a while with different approaches and this was the best I could come up with for developer ergonomics.

I tried this also:

export interface NextObserver<T> {
  isUnsubscribed?: boolean;
  next: (value: T) => void;
}

export interface ErrorObserver {
  isUnsubscribed?: boolean;
  error: (err: any) => void;
}

export interface CompletionObserver {
  isUnsubscribed?: boolean;
  complete: () => void;
}

export declare type Observer<T> = NextObserver<T> | ErrorObserver | CompletionObserver;

But that resulted in ugly experiences on the consumer end, where trying to call next on the Observer gave you TypeScript errors because "next" doesn't exist on " NextObserver | ErrorObserver | CompletionObserver"

[benlesh]

@kwonoj ... haha you typed that while I was adding additional comments. Yes, I did try that, but it ended up being ugly on the side of whatever consumes the observer. For example and error like: o.next does not exist on NextObserver<T> | ErrorObserver | CompleteObserver | (NextObserver<T> & ErrorObserver & CompleteObserver) | (ErrorObserver & CompleteObserver) | (NextObserver<T> & ErrorObserver) | (NextObserver<T> & CompleteObserver);

[kwonoj]

Yes, it'll be quite unfriendly from consumer perspective. For now I'd rather allow observer to be fully optional interfaces given that most cases caller wouldn't pass empty object ({}) in general. Course this is based on assumption there'll be better ergonomics support from typescript will be available. I hope microsoft/TypeScript#4889 can be answer.

[benlesh]

@kwonoj ... I got a solid answer from the TypeScript folks and reimplemented my solution. Now there is a PartialObserver<T> type that matches any partial observer. I've also updated all appropriate locations in code that could have partial observers.

[kwonoj]

Saw update from issue and this looks much better :) Change looks good to me.

p.s: just curious, how does error output now when partial observer does not have some interfaces? (as compare to above)

[benlesh]

@kwonoj if you try to pass an empty object, for example, to something that accepts PartialObserver<T>, it complains that {} does not match NextObserver<T> | ErrorObserver | CompletionObserver... likewise if there's too many additional methods: { next() {}, blah () {} }.

[kwonoj]

Thanks for clarification :)

[tetsuharuohzeki]

By the way, I think this guard will accepts IterableIterator<T> (from the code which have only a runtime dynamically type). In today's RxJS context, can a generator be a Observer<T>?

[benlesh]

@saneyuki ... not really... generators have next, throw and return methods. Very, very early versions of this library where implemented in that manner, but it was abandoned when the es-observable spec moved away from that.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.