Rxjs: takeUntil docs wrong.

[Dorus]

EDIT: (blesh) We need to update the docs

The docs should describe the behavior when the notifier passed to takeUntil does not emit, and instead completes. (for example, Observable.empty() or Observable.timer(100).ignoreElements())

---

Original Post

takeUntil on reactivex.io states

The TakeUntil subscribes and begins mirroring the source Observable. It also monitors a second Observable that you provide. If this second Observable emits an item or sends a termination notification, the Observable returned by TakeUntil stops mirroring the source Observable and terminates.

(emphasise is mine)

Similar, the docs state:

takeUntil subscribes and begins mirroring the source Observable. It also monitors a second Observable, notifier that you provide. If the notifier emits a value or a complete notification, the output Observable stops mirroring the source Observable and completes.

(emphasise is mine)

However, in the specs:

it('should take all values when notifier is empty', function () {

I think the correct behaviour would be to terminate when the notifier terminates, however at the very least the docs should be in line with the specs.

[frederikprijck]

Just for completion, a little code sample to reproduce:

Calling complete doesn't terminate.

const onDestroy$: Subject<any> = new Subject();

Rx.Observable.interval()
    .takeUntil(onDestroy$)
    .subscribe(console.log, undefined, () => console.log("complete"));

setTimeout(() => {
    onDestroy$.complete();
}, 5000);

Calling next does terminate.

const onDestroy$: Subject<any> = new Subject();

Rx.Observable.interval()
    .takeUntil(onDestroy$)
    .subscribe(console.log, undefined, () => console.log("complete"));

setTimeout(() => {
    onDestroy$.next();
}, 5000);

Edit:
For anyone having the need to make it work using complete, you can concate the onDestroy subject using making it terminate:

.takeUntil(onDestroy$.concat(Observable.of(0)))

[staltz]

Seems legit. I mean, I believe it should behave like you described. That said, RxJS v5 behaves just like RxJS v4 in this regard. So it has been like this for a very long time.

[Dorus]

For what it is worth. Rx.Net does not terminate on empty. RxJava does terminate on empty. That might explain why reactivex.io is confused on the subject.

[staltz]

And... FWIW, xstream equivalent to takeUntil is endWhen, and it completes the result stream when the notifier stream completes.

[frederikprijck]

And FWIW³: I found this issue on ReactiveCocoa.
Even tho I don't expect rxjs to be identical on an operator level, I did like the little conversation over there.

https://github.com/ReactiveCocoa/ReactiveCocoa/issues/1431

[jayphelps]

Hey! We discussed this today at our meeting, notes here. We'd like to keep the status quo as far as behavior, but certainly the docs could call out this behavior explicitly, to prevent any confusion.

[benlesh]

I've updated the title of this issue and edited the original post to include the intention of what we need to do to resolve the issue.

[Karasuni]

https://github.com/ReactiveX/rxjs/issues/2369

[ghost]

I ran into this as well. Please update the docs to match the behavior, or update the behavior to match the docs.

[jayphelps]

@errorx666 would you be interested in contributing?

[OliverJAsh]

Just stumbled into this, and then found this issue and this PR to fix the docs: https://github.com/ReactiveX/rxjs/pull/3035.

Unfortunately the docs don't seem to be updated yet. I guess this is waiting on the next release?

[insidewhy]

Just got burned by this also. Wonder how many other developers' time it's going to waste before the documentation finally gets updated ;)

[jayphelps]

@ohjames I don’t know, would you like to update it?

[OliverJAsh]

I just checked the docs at http://reactivex.io/rxjs/class/es6/Observable.js~Observable.html#instance-method-takeUntil, and they don't seem to be updated with the fix in https://github.com/ReactiveX/rxjs/pull/3035.

The docs on the site read:

If the notifier emits a value or a complete notification, the output Observable stops mirroring the source Observable and completes.

However, the PR changed this to:

If the notifier emits a value, the output Observable stops mirroring the source Observable and completes. If the notifier doesn't emit any value and completes then takeUntil will pass all values.

https://github.com/martinsik/rxjs/commit/a82121a164ad8f085bcd13883cb960953d302257#diff-61e6203a9881d218587e80e948e9aef1R23

@jayphelps Do you know why the site is not up-to-date with the docs in the code? I don't know how they're generated or updated.

[Karasuni]

@jayphelps I've asked last year how we can go about updating the docs. Just like @OliverJAsh says we don't know how these are generated or maintained. The docs have been completely incorrect wasting hours of developer's time for over a year now.

[benlesh]

@OliverJAsh @Karasuni ... The documentation build script has been broken for some time, it seems :( I manually updated the docs this morning and all is well.

Going forward, we have a much better documentation effort going on over at github.com/reactivex/rxjs-docs, lead by @ladyleet. They're still in-development, but the goal is to never have this problem again. It's all volunteer work, so if you feel passionately about the documentation for RxJS, I strongly encourage you to spent some time with those fine and talented folks.

It seems like we can close this one now.

[Dorus]

Possibly (hopefully) i'm jumping on this too soon, but http://reactivex.io/rxjs/class/es6/Observable.js~Observable.html#instance-method-takeUntil has not yet updated. Did the script already complete?

[benlesh]

@Dorus what's up there reflects what we have in stable. Any additional changes we'll need to do another PR.

[frederikprijck]

@benlesh this issue is about the takeuntil docs being wrong, not out of sync with the stable.

Sounds like a good idea to keep this open until the extra pr has been made + merged, as the take until docs are still wrong.

I can try to take a stab at it any time soon, if others want, feel free to do so.

[Karasuni]

@ReactiveX/rxjs-docs doesn't appear to be hosted anywhere. Are the docs being maintained separate of code then? I'd expect docs to be generated from the annotated code

[ladyleet]

@Karasuni it'll be a combination of the two once the docs project is complete - but yes the docs will be generated from the annotated code.

[maurocchi]

Recently I've observed the behaviour of an expression like empty().pipe(takeUntil(empty())) using rxjs v6 (illustrated in https://codepen.io/maurocchi/pen/MGROKQ?editors=0012), where the resulting observable does not complete.

Reading this ticket I've not understood if the missing completion event is expected or not, however in my opinion it should be considered as a bug.

[benlesh]

@maurocchi ... an empty observable doesn't emit a value, therefore it doesn't "notify". It's like subscribing to an event and never getting an event.

[benlesh]

I think we can close this issue, as the docs have moved and been updated since this was filed forever ago.