diff --git a/spec/operators/take-spec.ts b/spec/operators/take-spec.ts index 07c80dc096..e18fdfa443 100644 --- a/spec/operators/take-spec.ts +++ b/spec/operators/take-spec.ts @@ -4,8 +4,6 @@ import { range, ArgumentOutOfRangeError, of, Observable, Subject } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { observableMatcher } from '../helpers/observableMatcher'; -declare function asDiagram(arg: string): Function; - /** @test {take} */ describe('take operator', () => { let testScheduler: TestScheduler; @@ -14,7 +12,28 @@ describe('take operator', () => { testScheduler = new TestScheduler(observableMatcher); }); - asDiagram('take(2)')('should take two values of an observable with many values', () => { + it('should error when a non-number is passed to it, or when no argument is passed (Non-TS case)', () => { + expect(() => { + of(1, 2, 3).pipe( + (take as any)() + ); + }).to.throw(TypeError, `'count' is not a number`); + + expect(() => { + of(1, 2, 3).pipe( + (take as any)('banana') + ); + }).to.throw(TypeError, `'count' is not a number`); + + // Standard type coersion behavior in JS. + expect(() => { + of(1, 2, 3).pipe( + (take as any)('1') + ); + }).not.to.throw(); + }); + + it('should take two values of an observable with many values', () => { testScheduler.run(({ cold, expectObservable, expectSubscriptions }) => { const e1 = cold(' --a-----b----c---d--|'); const e1subs = ' ^-------!------------'; diff --git a/spec/operators/takeLast-spec.ts b/spec/operators/takeLast-spec.ts index 95a3927748..009aa92c3e 100644 --- a/spec/operators/takeLast-spec.ts +++ b/spec/operators/takeLast-spec.ts @@ -1,164 +1,216 @@ import { expect } from 'chai'; -import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; import { takeLast, mergeMap } from 'rxjs/operators'; import { range, ArgumentOutOfRangeError, of } from 'rxjs'; - -declare function asDiagram(arg: string): Function; +import { TestScheduler } from 'rxjs/testing'; +import { assertDeepEquals } from '../helpers/test-helper'; /** @test {takeLast} */ describe('takeLast operator', () => { - asDiagram('takeLast(2)')('should take two values of an observable with many values', () => { - const e1 = cold('--a-----b----c---d--| '); - const e1subs = '^ ! '; - const expected = '--------------------(cd|)'; + let rxTest: TestScheduler; + + beforeEach(() => { + 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--| '); + const e1subs = ' ^-------------------! '; + const expected = '--------------------(cd|)'; - expectObservable(e1.pipe(takeLast(2))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(2))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should take last three values', () => { - const e1 = cold('--a-----b----c---d--| '); - const e1subs = '^ ! '; - const expected = '--------------------(bcd|)'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' --a-----b----c---d--| '); + const e1subs = ' ^-------------------! '; + const expected = '--------------------(bcd|)'; - expectObservable(e1.pipe(takeLast(3))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(3))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should take all element when try to take larger then source', () => { - const e1 = cold('--a-----b----c---d--| '); - const e1subs = '^ ! '; - const expected = '--------------------(abcd|)'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' --a-----b----c---d--| '); + const e1subs = ' ^-------------------! '; + const expected = '--------------------(abcd|)'; - expectObservable(e1.pipe(takeLast(5))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(5))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should take all element when try to take exact', () => { - const e1 = cold('--a-----b----c---d--| '); - const e1subs = '^ ! '; - const expected = '--------------------(abcd|)'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' --a-----b----c---d--| '); + const e1subs = ' ^-------------------! '; + const expected = '--------------------(abcd|)'; - expectObservable(e1.pipe(takeLast(4))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(4))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should not take any values', () => { - const e1 = cold('--a-----b----c---d--|'); - const expected = '|'; + rxTest.run(({ cold, expectObservable }) => { + const e1 = cold(' --a-----b----c---d--|'); + const expected = '|'; - expectObservable(e1.pipe(takeLast(0))).toBe(expected); + expectObservable(e1.pipe(takeLast(0))).toBe(expected); + }); }); it('should work with empty', () => { - const e1 = cold('|'); - const e1subs = '(^!)'; - const expected = '|'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' |'); + const e1subs = ' (^!)'; + const expected = '|'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should go on forever on never', () => { - const e1 = cold('-'); - const e1subs = '^'; - const expected = '-'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' -'); + const e1subs = ' ^'; + const expected = '-'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should be empty on takeLast(0)', () => { - const e1 = hot('--a--^--b----c---d--|'); - const e1subs: string[] = []; // Don't subscribe at all - const expected = '|'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('--a--^--b----c---d--|'); + const expected = ' |'; + const e1subs: string[] = []; // Don't subscribe at all - expectObservable(e1.pipe(takeLast(0))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(0))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should take one value from an observable with one value', () => { - const e1 = hot('---(a|)'); - const e1subs = '^ ! '; - const expected = '---(a|)'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot(' ---(a|)'); + const e1subs = ' ^--! '; + const expected = '---(a|)'; - expectObservable(e1.pipe(takeLast(1))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(1))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should take one value from an observable with many values', () => { - const e1 = hot('--a--^--b----c---d--| '); - const e1subs = '^ ! '; - const expected = '---------------(d|)'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('--a--^--b----c---d--| '); + const e1subs = ' ^--------------! '; + const expected = ' ---------------(d|)'; - expectObservable(e1.pipe(takeLast(1))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(1))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should error on empty', () => { - const e1 = hot('--a--^----|'); - const e1subs = '^ !'; - const expected = '-----|'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('--a--^----|'); + const e1subs = ' ^----!'; + const expected = ' -----|'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should propagate error from the source observable', () => { - const e1 = hot('---^---#', undefined, 'too bad'); - const e1subs = '^ !'; - const expected = '----#'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('---^---#', undefined, 'too bad'); + const e1subs = ' ^---!'; + const expected = ' ----#'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected, null, 'too bad'); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected, null, 'too bad'); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should propagate error from an observable with values', () => { - const e1 = hot('---^--a--b--#'); - const e1subs = '^ !'; - const expected = '---------#'; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('---^--a--b--#'); + const e1subs = ' ^--------!'; + const expected = ' ---------#'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should allow unsubscribing explicitly and early', () => { - const e1 = hot('---^--a--b-----c--d--e--|'); - const unsub = ' ! '; - const e1subs = '^ ! '; - const expected = '---------- '; + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('---^--a--b-----c--d--e--|'); + const unsub = ' ---------! '; + const e1subs = ' ^--------! '; + const expected = ' ----------------------'; - expectObservable(e1.pipe(takeLast(42)), unsub).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42)), unsub).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should work with throw', () => { - const e1 = cold('#'); - const e1subs = '(^!)'; - const expected = '#'; + rxTest.run(({ cold, expectObservable, expectSubscriptions }) => { + const e1 = cold(' #'); + const e1subs = ' (^!)'; + const expected = '#'; - expectObservable(e1.pipe(takeLast(42))).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectObservable(e1.pipe(takeLast(42))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); it('should throw if total is less than zero', () => { - expect(() => { range(0, 10).pipe(takeLast(-1)); }) - .to.throw(ArgumentOutOfRangeError); + expect(() => { + range(0, 10).pipe(takeLast(-1)); + }).to.throw(ArgumentOutOfRangeError); }); it('should not break unsubscription chain when unsubscribed explicitly', () => { - const e1 = hot('---^--a--b-----c--d--e--|'); - const unsub = ' ! '; - const e1subs = '^ ! '; - const expected = '---------- '; - - const result = e1.pipe( - mergeMap((x: string) => of(x)), - takeLast(42), - mergeMap((x: string) => of(x)) - ); - - expectObservable(result, unsub).toBe(expected); - expectSubscriptions(e1.subscriptions).toBe(e1subs); + rxTest.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot('---^--a--b-----c--d--e--|'); + const unsub = ' ---------! '; + const e1subs = ' ^--------! '; + const expected = ' ----------------------'; + + const result = e1.pipe( + mergeMap((x: string) => of(x)), + takeLast(42), + mergeMap((x: string) => of(x)) + ); + + expectObservable(result, unsub).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + }); }); }); diff --git a/src/internal/operators/take.ts b/src/internal/operators/take.ts index 7058c13f3d..c525b020fd 100644 --- a/src/internal/operators/take.ts +++ b/src/internal/operators/take.ts @@ -1,9 +1,9 @@ import { Operator } from '../Operator'; import { Subscriber } from '../Subscriber'; import { ArgumentOutOfRangeError } from '../util/ArgumentOutOfRangeError'; -import { EMPTY } from '../observable/empty'; import { Observable } from '../Observable'; import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; +import { EMPTY } from '../observable/empty'; /** * Emits only the first `count` values emitted by the source Observable. @@ -43,50 +43,42 @@ import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; * * @throws {ArgumentOutOfRangeError} When using `take(i)`, it delivers an * ArgumentOutOrRangeError to the Observer's `error` callback if `i < 0`. - * - * @param {number} count The maximum number of `next` values to emit. - * @return {Observable} An Observable that emits only the first `count` + * @throws {TypeError} when no numeric argument is passed. + * @param count The maximum number of `next` values to emit. + * @return An Observable that emits only the first `count` * values emitted by the source Observable, or all of the values from the source * if the source emits fewer than `count` values. - * @name take */ export function take(count: number): MonoTypeOperatorFunction { - return (source: Observable) => { - if (count === 0) { - return EMPTY; - } else { - return source.lift(new TakeOperator(count)); - } - }; + if (isNaN(count)) { + throw new TypeError(`'count' is not a number`); + } + if (count < 0) { + throw new ArgumentOutOfRangeError; + } + + return (source: Observable) => (count === 0) ? EMPTY : source.lift(new TakeOperator(count)); } class TakeOperator implements Operator { - constructor(private total: number) { - if (this.total < 0) { - throw new ArgumentOutOfRangeError; - } + constructor(private count: number) { } call(subscriber: Subscriber, source: any): TeardownLogic { - return source.subscribe(new TakeSubscriber(subscriber, this.total)); + return source.subscribe(new TakeSubscriber(subscriber, this.count)); } } -/** - * We need this JSDoc comment for affecting ESDoc. - * @ignore - * @extends {Ignored} - */ class TakeSubscriber extends Subscriber { - private count: number = 0; + private _valueCount: number = 0; - constructor(destination: Subscriber, private total: number) { + constructor(destination: Subscriber, private count: number) { super(destination); } protected _next(value: T): void { - const total = this.total; - const count = ++this.count; + const total = this.count; + const count = ++this._valueCount; if (count <= total) { this.destination.next(value); if (count === total) { diff --git a/src/internal/operators/takeLast.ts b/src/internal/operators/takeLast.ts index aff9d4204c..36c77d0bb7 100644 --- a/src/internal/operators/takeLast.ts +++ b/src/internal/operators/takeLast.ts @@ -39,14 +39,21 @@ 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. - * @name takeLast */ 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; @@ -58,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 { @@ -68,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;