The Operator Creation doc suggests poor practices and doesn't support TS

[slikts]

The Operator Creation doc currently suggests using a stage-2 proposal feature (bind syntax) or extending Observable.prototype; both are poor practices, since there's no guarantee that a stage-2 feature will be included in the language, and using the prototype of an external library as a global namespace is a way to create name collisions. There should at least be caveats about issues these approaches may cause.

The third listed method for adding custom operators (overriding lift()) doesn't work well in TS (#1876); there should be details of how to properly deal with a common use case like adding custom operators when using the language the library itself is written it.

I'd suggest removing the mention of bind syntax and adding a warning against extending Observable.prototype, but I don't know what a workaround to the TS issue would be; would really like to know that myself.

[masaeedu]

@slikts Given that monkey patching things into the Observable prototype is how the modules supplying the core operators are implemented as well, I'd say that is the standard approach for implementing operators at the moment. It isn't great, and you have to watch out for conflicts if you're implementing operators or importing them, but that's where we're at right now. Perhaps the docs should mention that your module should check for conflicts before patching the prototype and throw an error if one is present.

The cleaner alternative is to use function bind, which can be used without the proposed :: syntax sugar, but for a variety of reasons it is pretty inconvenient right now. You can use the built in Function prototype methods, i.e. youroperator.apply or youroperator.bind, but this means you have to sacrifice good type checking.

Alternatively, you can write a well typed helper function like apply and use something like apply(youroperator, observable), but the problem then is that chaining operators becomes awkward. You can improve chainability by using a curried API like apply(youroperator)(observable), which then allows you to use function composition on a sequence of operators, but when using function composition you run up against limitations in TypeScript's type inference.

All in all it's a bit of a pickle. My personal preference is to use the currying+composition approach, and bail out the type system wherever necessary. I do think that this is a little bit too much overhead to expect most users to tolerate however, so the standard approach remains monkey patching.

[slikts]

Observable.prototype naming conflicts don't apply to RxJS itself, since it owns that namespace; the conflicts can arise by others using it as a global namespace, hence it's a poor practice that should be warned against.

Overriding lift() is currently the best way to extend Observable in JS, but it doesn't work properly with TS, which should be added as a caveat to the docs. The specific issue is that TypeScript doesn't support higher-kinded types (microsoft/TypeScript#1213).

Would you have an example of how the apply() function you mention would be implemented and used?

I think a viable workaround for the lack of HKT in TS could be generating declarations to override return types for methods inherited from Observable; for example:

class Foo<T> extends Observable<T> {
  lift(...): {
    // ... override lift()
  }
}

Then generate declarations like this:

interface Foo<T> {
  refCount(): Foo<T>
  merge(): Foo<T>
  // etc. for every method that would return Observable<T>
}

[jooyunghan]

There is another problem in the custom operator doc. Let's create a simple custom operator which just pass values to downstream.

Observable.prototype.op = function op() {
  return Observable.create(subscriber => {
    return this.subscribe(value => subscriber.next(value));
  });
};
Observable.range(0,3).do(console.log).op().take(1).subscribe(console.log);

Surprisingly the output is ...

0
0
1
2

... which should be ...

0
0

This wrong behavior can cause a problem.(What if range(1, INFINITE) is used?)

The reason is because the Subscriber created by toSubscriber doesn't add itself to the downstream subscribe and the subscription returned from inner subscribe is also not used due to there is no associated Schedulers.

To workaround this wrong behavior, a custom operator should be implemented in the way which the standard operators are implemented. That is, its own subscriber class.

class OpSubscriber extends Subscriber {
  constructor(destination) {
    super(destination); // this subscriber is added to destination
  }
  // override _next if necessary
};

Observable.prototype.op = function op() {
  return Observable.create(subscriber => {
    return this.subscribe(new OpSubscriber(subscriber));
  });
};

Observable.range(0,3).do(console.log).op().take(1).subscribe(console.log);

Or using `lift()

class OpSubscriber extends Subscriber {
  constructor(destination) {
    super(destination); // this subscriber is added to destination
  }
  // override _next if necessary
};

class OpOperator {
  call(destination, source) {
    return source.subscribe(new OpSubscriber(destination));
  }
}

Observable.prototype.op = function op() {
  return this.lift(new OpOperator());
};

Observable.range(0,3).do(console.log).op().take(1).subscribe(console.log);

If you still want to use a simpler PartialObserver style, you should use Subscribe.create and add a subscriber to downstream subscriber explicitly.

Observable.prototype.op = function op() {
  return Observable.create(downstream => {
    let subscriber = Subscriber.create(value => downstream.next(value));
    downstream.add(subscriber);
    return this.subscribe(subscriber);
  });
};

I think this is a kind of a bug in that creating a subscriber from PartialObserver fails to chain subscriptions. Note that when subscribers created from PartialObserver are used in the last (not in the middle, e.g. a custom operator) of the chain or there is a scheduler attached then everything is fine.

[benlesh]

This has changed with pipeable operators.