Rxjs: Take operator not completing if count is a string

[christophbuehler]

Bug Report

Current Behavior
When using the take operator and providing a string instead of a number for the first and only argument ("count"), everything behaves the same way with the exception of the observable not completing after emitting n times.

Reproduction
https://stackblitz.com/edit/rxjs-kenahb?devtoolsheight=60

const mouseClick = fromEvent(window, 'click');
mouseClick.pipe(
  take('2'),
  tap(() => console.log('CLICK')),
  finalize(() => console.log('completed'))
).subscribe();

Expected behavior
The above code should write 'CLICK', 'CLICK' and 'completed' to the console after clicking twice. Because '2' is a string, the output is 'CLICK', 'CLICK'.

Environment

• Runtime: Chrome
• RxJS version: 6.6.3

[kwonoj]

I'm not sure if I understood issue correctly, take explicitly requires to supply number https://github.com/ReactiveX/rxjs/blob/master/src/internal/operators/take.ts#L53 . Are you saying it should try to parse given argument as number if string is provided and it's number?

[christophbuehler]

I'm not sure if I understood issue correctly, take explicitly requires to supply number https://github.com/ReactiveX/rxjs/blob/master/src/internal/operators/take.ts#L53 . Are you saying it should try to parse given argument as number if string is provided and it's number?

Exactly. It kind of does that already, but completed is never called. Took me forever to find out that this was the issue. It should either work as expected or not work at all.

[christophbuehler]

The real code that caused this issue without raising any alarm bells:

connect(inputs: Observable<any>[]) {
  const taken = inputs[1].pipe(
    first(),
    switchMap((count: number) => inputs[0].pipe(take(count))),
  );
  return [taken];
}

count ended up not being a number in runtime.

[cartant]

It's because === is used in the v6 implementation:

https://github.com/ReactiveX/rxjs/blob/6.x/src/internal/operators/take.ts#L93

This won't happen in v7 - which is the implementation linked in a comment above.

[christophbuehler]

Perfect, thanks!

[christophbuehler]

@cartant Can I close this issue?

[cartant]

@christophbuehler Leave it open for the moment, I think. The team can decide what to do with it tomorrow.

[benlesh]

Core Team Meeting: We'll deal with these on a case-by-case basis. Adding a test at the time. We will not be adding runtime type checking that throws.