the defaultIfEmpty operator has erroneous & inconvenient typing

[backbone87]

Bug Report

Current Behavior

const a = of(true).pipe(defaultIfEmpty()); // a is Observable<boolean>, but should be Observable<boolean | null>
const b = of(undefined).pipe(defaultIfEmpty(undefined)); // b is Observable<undefined>, but should be Observable<undefined | null>

const c = of(true).pipe(defaultIfEmpty(null));
const d: Observable<boolean | null> = of(true).pipe(defaultIfEmpty(null));
// c & d both report the same error at the defaultIfEmpty operator:
/*
Argument of type 'MonoTypeOperatorFunction<null>' is not assignable to parameter of type 'OperatorFunction<boolean, null>'.
  Types of parameters 'source' and 'source' are incompatible.
    Type 'Observable<boolean>' is not assignable to type 'Observable<null>'.
      Type 'boolean' is not assignable to type 'null'.ts(2345)
*/

Expected behavior

const a = of(true).pipe(defaultIfEmpty()); // a is Observable<boolean | null>
const b = of(undefined).pipe(defaultIfEmpty(undefined)); // b is Observable<undefined | null>

const c = of(true).pipe(defaultIfEmpty(null)); // no error, c is Observable<boolean | null>
const d: Observable<boolean | null> = of(true).pipe(defaultIfEmpty(null)); // no error, d is Observable<boolean | null>

Reproduction
Creating a repo with stackblitz or codesandbox didnt work as changing the TS version/config doesnt seem to affect their editor.
You an just take the code above and drop it into any TS ^4 strict project.

Environment

• RxJS version: ^6
• TypeScript: ^4

Possible Solution
change the type signatur of defaultIfEmpty to something like:
<T, R>(value?: R) => OperatorFunction<T, T | R>

Additional context/Screenshots
I would also deprecate the default of value and require an argument:
<T, R>(value: R) => OperatorFunction<T, T | R>
this is because defaultIfEmpty(undefined) results actually in the same as defaultIfEmpty(null), which is unexpected.

[benlesh]

The types might not be correct, but I think I disagree with the expectations here.

I think it should be:

const a  = of(true).pipe(defaultIfEmpty()); // $ExpectError (defaultIfEmpty should require an argument)
const b = of(undefined).pipe(defaultIfEmpty(undefined)); // $ExpectType Observable<undefined> (if we provide an undefined here, it should be undefined, not null)

Further, the Observable<never> sources should work properly:

const a = EMPTY.pipe(defaultIfEmpty(123)); // $ExpectType Observable<number>

This last one should work as you expect though:

const d: Observable<boolean | null> = of(true).pipe(defaultIfEmpty(null)); // $ExpectType Observable<boolean | null>

[backbone87]

but I think I disagree with the expectations here

i dont think we disagree at all, if you read my last point under "additional context". but this would be a breaking change. the typing issues should be fixed asap to reflect the current reality, which i showed in the "expected behavior" section.

for the next major i 100% agree with your proposals.

edit: so this ticket is really only about fixing incorrect types. cleaning up the API & behavior would be a change request, which i can create a separate ticket for, if you like.

[benlesh]

@backbone87 thanks for bring this to my attention. As it turns out, it was related to a rash of edge case bugs related to the composition of this operator: #6070

[backbone87]

so #6070 goes into ^7 since breaking. can we fix the typings for ^6, should i give it a go?