Is this implementation of endWith correct

[CarstenLeue]

Bug Report

rxjs 6.5.1 implements endWith as follows:

export function endWith<T>(...array: Array<T | SchedulerLike>): MonoTypeOperatorFunction<T> {
  return (source: Observable<T>) => concat(source, ...(array as any[])) as Observable<T>;
}

Is this really correct given that concat expects observables as input, but array carries the individual items?

[daka1510]

I've also hit issues with the endWith operator. Here's a simple jest test case that works fine with 6.4.0 but fails with 6.5.1.

import { of } from 'rxjs';
import { endWith, tap } from 'rxjs/operators';

it('endWith operator: sample failing test', () => {
  return of('1')
    .pipe(
      endWith({}),
      tap((value) => console.log(value))
    )
    .toPromise();
});

Here's the console output when running this test with rxjs 6.5.1:

 console.log src/endWith.spec.ts:8
1

TypeError: You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.

at Object.<anonymous>.exports.subscribeTo (/node_modules/rxjs/src/internal/util/subscribeTo.ts:27:11)
at Object.subscribeToResult (/node_modules/rxjs/src/internal/util/subscribeToResult.ts:28:10)
at MergeMapSubscriber.Object.<anonymous>.MergeMapSubscriber._innerSub (/node_modules/rxjs/src/internal/operators/mergeMap.ts:148:5)
at MergeMapSubscriber.Object.<anonymous>.MergeMapSubscriber._tryNext (/node_modules/rxjs/src/internal/operators/mergeMap.ts:141:10)
at MergeMapSubscriber.Object.<anonymous>.MergeMapSubscriber._next (/node_modules/rxjs/src/internal/operators/mergeMap.ts:125:12)
at MergeMapSubscriber.Object.<anonymous>.Subscriber.next (/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at Observable._subscribe (/node_modules/rxjs/src/internal/util/subscribeToArray.ts:9:16)
at Observable.Object.<anonymous>.Observable._trySubscribe (/node_modules/rxjs/src/internal/Observable.ts:238:19)
at Observable.Object.<anonymous>.Observable.subscribe (/node_modules/rxjs/src/internal/Observable.ts:219:14)
at MergeMapOperator.Object.<anonymous>.MergeMapOperator.call (/node_modules/rxjs/src/internal/operators/mergeMap.ts:100:19)

[CarstenLeue]

I would have expected this to read:

export function endWith<T>(...array: Array<T | SchedulerLike>): MonoTypeOperatorFunction<T> {
  return (source: Observable<T>) => concat(source, from(array)) as Observable<T>;
}

or shorter

export function endWith<T>(...array: Array<T | SchedulerLike>): MonoTypeOperatorFunction<T> {
 const from$ = from(arguments);
  return (source: Observable<T>) => concat(source, from$) as Observable<T>;
}

[cartant]

@CarstenLeue Looks like a bug, to me. But the fix is a little more subtle, as the array could contain a Scheduler. The array will need to be spread into a call to of, instead of being passed to from.

[cartant]

This is a really interesting bug. The tests pass because strings are iterable - so they are valid observable inputs. If the tests had used values other than strings, they would have failed!

[felixfbecker]

Met the same bug, was caught by a test for us

[2color]

Experiencing the same problem: https://jsbin.com/sasumoy/edit?html,js,console

[cartant]

Fixed in #4735