Fix take and takeLast

[benlesh]

Add runtime assertions for JavaScript usage, where invalid argument may be passed causing strange behavior.

BREAKING CHANGE: take will throw a TypeError if called as take() or with an isNaN argument passed.

BREAKING CHANGE: takeLast will throw a TypeError if called as takeLast() or with an isNaN argument passed.

[cartant]

IDK what the implications of removing this are.

[benlesh]

Nothing, really. I can be put back, but as of right now, we don't run the diagram generation anymore.

[benlesh]

We'll tackle these in another PR, cc @niklas-wortmann

[cartant]

I think we should use Number.isNaN here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN

[benlesh]

Well, not really, Number.isNaN would only return true if it were actually NaN. We want to account for strings that are valid numbers as well. There are people that use RxJS with plain JavaScript (like psychopaths!) and this should be valid for them:

const input = document.querySelector('input[type="text"]');

of(1, 2, 3, 4).pipe(take(input.value)).subscribe(x => console.log(x));

DOES THAT CODE SUCK? Yes. Yes it does. But JS has coercion as a feature. 🤷‍♂

[cartant]

I think we should use Number.isNaN here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN

[benlesh]

IIRC, that would only return true if it's actually NaN. People could pass in 'blah' there and it would be false if we used Number.isNaN.

[kievsash]

is it expected behavior not to complete if the operator throws because it is used inappropriately?
Actually yeah, too much overhead if to move all checking logic to TakeSubscriber constructor

export function take<T>(count: number): MonoTypeOperatorFunction<T> {
//   if (isNaN(count)) {
//     throw new TypeError(`'count' is not a number`);
//   }
//   if (count < 0) {
//     throw new ArgumentOutOfRangeError;
//   }

  return (source: Observable<T>) =>source.lift(new TakeOperator(count));
}

class TakeOperator<T> implements Operator<T, T> {
  constructor(private count: number) {
  }

  call(subscriber: Subscriber<T>, source: any): TeardownLogic {
    return source.subscribe(new TakeSubscriber(subscriber, this.count));
  }
}

class TakeSubscriber<T> extends Subscriber<T> {
  private _valueCount: number = 0;

  constructor(destination: Subscriber<T>, private count: number) {
    super(destination);

    if (isNaN(count)) {
        this.destination.complete()
        throw new TypeError(`'count' is not a number`);
    }
    if (count < 0) {
        this.destination.complete()
        throw new ArgumentOutOfRangeError;
    }

    if (count === 0) {
        this.destination.complete() // EMPTY
    }
  }