From 6828bb5fd544fcca1469a1c06d2ccca1ecede4c8 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Wed, 26 Feb 2020 17:31:21 -0600 Subject: [PATCH] fix(takeLast): add runtime assertions for invalid arguments BREAKING CHANGE: Calling takeLast without arguments or with an argument that is NaN will throw a TypeError --- spec/operators/takeLast-spec.ts | 14 ++++++++++++++ src/internal/operators/takeLast.ts | 26 ++++++++++++-------------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/spec/operators/takeLast-spec.ts b/spec/operators/takeLast-spec.ts index ec2d9e01920..009aa92c3eb 100644 --- a/spec/operators/takeLast-spec.ts +++ b/spec/operators/takeLast-spec.ts @@ -12,6 +12,20 @@ describe('takeLast operator', () => { rxTest = new TestScheduler(assertDeepEquals); }); + it('should error for invalid arguments', () => { + expect(() => { + of(1, 2, 3).pipe((takeLast as any)()); + }).to.throw(TypeError, `'count' is not a number`); + + expect(() => { + of(1, 2, 3).pipe((takeLast as any)('banana')); + }).to.throw(TypeError, `'count' is not a number`); + + expect(() => { + of(1, 2, 3).pipe((takeLast as any)('3')); + }).not.to.throw(); + }); + it('should take two values of an observable with many values', () => { rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { const e1 = cold('--a-----b----c---d--| '); diff --git a/src/internal/operators/takeLast.ts b/src/internal/operators/takeLast.ts index 5d215f64fe4..36c77d0bb7b 100644 --- a/src/internal/operators/takeLast.ts +++ b/src/internal/operators/takeLast.ts @@ -1,7 +1,7 @@ import { Operator } from '../Operator'; import { Subscriber } from '../Subscriber'; import { ArgumentOutOfRangeError } from '../util/ArgumentOutOfRangeError'; -import { empty } from '../observable/empty'; +import { EMPTY } from '../observable/empty'; import { Observable } from '../Observable'; import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; @@ -39,18 +39,24 @@ import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; * * @throws {ArgumentOutOfRangeError} When using `takeLast(i)`, it delivers an * ArgumentOutOrRangeError to the Observer's `error` callback if `i < 0`. + * @throws {TypeError} If the count is not provided or is not a number. * - * @param {number} count The maximum number of values to emit from the end of + * @param count The maximum number of values to emit from the end of * the sequence of values emitted by the source Observable. - * @return {Observable} An Observable that emits at most the last count + * @return An Observable that emits at most the last count * values emitted by the source Observable. - * @method takeLast - * @owner Observable */ export function takeLast(count: number): MonoTypeOperatorFunction { + if (isNaN(count)) { + throw new TypeError(`'count' is not a number`); + } + if (count < 0) { + throw new ArgumentOutOfRangeError; + } + return function takeLastOperatorFunction(source: Observable): Observable { if (count === 0) { - return empty(); + return EMPTY; } else { return source.lift(new TakeLastOperator(count)); } @@ -59,9 +65,6 @@ export function takeLast(count: number): MonoTypeOperatorFunction { class TakeLastOperator implements Operator { constructor(private total: number) { - if (this.total < 0) { - throw new ArgumentOutOfRangeError; - } } call(subscriber: Subscriber, source: any): TeardownLogic { @@ -69,11 +72,6 @@ class TakeLastOperator implements Operator { } } -/** - * We need this JSDoc comment for affecting ESDoc. - * @ignore - * @extends {Ignored} - */ class TakeLastSubscriber extends Subscriber { private ring: Array = new Array(); private count: number = 0;