delayWhen should use an indefinite delay for notifiers that complete without emitting

[cartant]

This issue's title has been changed, as the implementation of delayWhen is not correct - see the discussion below.

The original issue's description follows.

RxJS version: 6.1.0

Code to reproduce:

import { EMPTY, of } from "rxjs";
import { delay, delayWhen } from "rxjs/operators";

of(0).pipe(
    delayWhen(() => EMPTY.pipe(delay(0)))
).subscribe(console.log);

Expected behavior:

0 should be emitted and logged to the console.

Actual behavior:

Nothing is emitted and logged to the console.

If the of function is passed 1 instead of 0, 1 is logged to the console.

Additional information:

Looking at delayWhen, I can see another problem (other than #3663) with the implementation. If the value is falsy, and the notifier completes - rather than nexts - the value won't be emitted.

[benlesh]

So the semantics here seem off... The notifier completes without notifying, therefor the delay should last indefinitely. This isn't a bug with the library so much as it is with the usage.

[Airblader]

@benlesh The docs explicitly state that the notifier completing should cause the value to be emitted.

[benlesh]

🤦‍♂️

[benlesh]

Okay, so we need to fix the issue, but deprecate the behavior.

[Hyumiris]

I have recently run into the same issue(s).

Regarding the deprecation

Even though there is a deprecation warning for Observable<never> this only works if the generated Observable is known to be of type never at compile time.
In every other case the deprecation warning isn't shown and the official documentation still doesn't mention anything about the depreciation.

Regarding the bug

The bug that the behaviour of the delayWhen operator depends on the truthiness of the source value persists as well.

Here is a more concise version of the reproduction code:

import { of, EMPTY } from 'rxjs'; 
import { delayWhen } from 'rxjs/operators';

of(true, false, 0, 1).pipe(
  delayWhen(() => EMPTY)
).subscribe(console.log);

// this logs:
// >> true
// >> 1

[BioPhoton]

@cartant how can we move forward with this?
Is this PR teambition/teambition-sdk#576 solving the iisue?

[xumepadismal]

Hi guys! One more thought regarding the deprecation

Can someone explain to me why the deprecated version of delayWhen uses Observable<never> instead of Observable<void>? All the discussions is about notifiers that "complete without emit" whereas Observable<never> === "emit no items, never complete"...

[Airblader]

The generic is the type of the emitted values. void means it emits but without data, never means it never emits.

[xumepadismal]

Ah got it... Thanks @Airblader!

So EMPTY and NEVER has identical type signatures and there is no way to distinguish them and that's why we get the problem described by @Hyumiris (about deprecation). Is it correct?

[benlesh]

This is still an issue and we should resolve it before we release v7, IMO.

[roger6106]

I've just spent a while tracking down an issue related to this. It appears that the documentation was not updated:

The source value is emitted on the output Observable only when the duration Observable emits a value or completes.

[ducthang-vu]

This looks to be still an open issue. Any solution?

[roger6106]

No one appears to be looking at this issue, so I've opened a new issue for it: #6977.