-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(skipLast): add skipLast operator #2316
Changes from 3 commits
47f5752
0940b14
34eab36
83ed187
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
var RxOld = require('rx'); | ||
var RxNew = require('../../../../index'); | ||
|
||
module.exports = function (suite) { | ||
var oldSkipLastWithImmediateScheduler = RxOld.Observable.range(0, 500, RxOld.Scheduler.currentThread).skipLast(50); | ||
var newSkipLastWithImmediateScheduler = RxNew.Observable.range(0, 500, RxNew.Scheduler.queue).skipLast(50); | ||
|
||
function _next(x) { } | ||
function _error(e) { } | ||
function _complete() { } | ||
return suite | ||
.add('old skipLast with current thread scheduler', function () { | ||
oldSkipLastWithImmediateScheduler.subscribe(_next, _error, _complete); | ||
}) | ||
.add('new skipLast with current thread scheduler', function () { | ||
newSkipLastWithImmediateScheduler.subscribe(_next, _error, _complete); | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
var RxOld = require('rx'); | ||
var RxNew = require('../../../../index'); | ||
|
||
module.exports = function (suite) { | ||
var oldSkipLastWithImmediateScheduler = RxOld.Observable.range(0, 500, RxOld.Scheduler.immediate).skipLast(50); | ||
var newSkipLastWithImmediateScheduler = RxNew.Observable.range(0, 500).skipLast(50); | ||
|
||
function _next(x) { } | ||
function _error(e) { } | ||
function _complete() { } | ||
return suite | ||
.add('old skipLast with immediate scheduler', function () { | ||
oldSkipLastWithImmediateScheduler.subscribe(_next, _error, _complete); | ||
}) | ||
.add('new skipLast with immediate scheduler', function () { | ||
newSkipLastWithImmediateScheduler.subscribe(_next, _error, _complete); | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
import {expect} from 'chai'; | ||
import * as Rx from '../../dist/cjs/Rx'; | ||
declare const {hot, cold, asDiagram, expectObservable, expectSubscriptions}; | ||
|
||
const Observable = Rx.Observable; | ||
|
||
/** @test {takeLast} */ | ||
describe('Observable.prototype.skipLast', () => { | ||
asDiagram('skipLast(2)')('should skip two values of an observable with many values', () => { | ||
const e1 = cold('--a-----b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '-------------a---b--|'; | ||
|
||
expectObservable(e1.skipLast(2)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should skip last three values', () => { | ||
const e1 = cold('--a-----b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '-----------------a--|'; | ||
|
||
expectObservable(e1.skipLast(3)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should skip all values when trying to take larger then source', () => { | ||
const e1 = cold('--a-----b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '--------------------|'; | ||
|
||
expectObservable(e1.skipLast(5)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should skip all element when try to take exact', () => { | ||
const e1 = cold('--a-----b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '--------------------|'; | ||
|
||
expectObservable(e1.skipLast(4)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should not skip any values', () => { | ||
const e1 = cold('--a-----b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '--a-----b----c---d--|'; | ||
|
||
expectObservable(e1.skipLast(0)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should work with empty', () => { | ||
const e1 = cold('|'); | ||
const e1subs = '(^!)'; | ||
const expected = '|'; | ||
|
||
expectObservable(e1.skipLast(42)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should go on forever on never', () => { | ||
const e1 = cold('-'); | ||
const e1subs = '^'; | ||
const expected = '-'; | ||
|
||
expectObservable(e1.skipLast(42)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should skip one value from an observable with one value', () => { | ||
const e1 = hot('---(a|)'); | ||
const e1subs = '^ ! '; | ||
const expected = '---| '; | ||
|
||
expectObservable(e1.skipLast(1)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should skip one value from an observable with many values', () => { | ||
const e1 = hot('--a--^--b----c---d--|'); | ||
const e1subs = '^ !'; | ||
const expected = '--------b---c--|'; | ||
|
||
expectObservable(e1.skipLast(1)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should work with empty and early emission', () => { | ||
const e1 = hot('--a--^----|'); | ||
const e1subs = '^ !'; | ||
const expected = '-----|'; | ||
|
||
expectObservable(e1.skipLast(42)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should propagate error from the source observable', () => { | ||
const e1 = hot('---^---#', null, 'too bad'); | ||
const e1subs = '^ !'; | ||
const expected = '----#'; | ||
|
||
expectObservable(e1.skipLast(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 = '---------#'; | ||
|
||
expectObservable(e1.skipLast(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 = '---------- '; | ||
|
||
expectObservable(e1.skipLast(42), unsub).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should work with throw', () => { | ||
const e1 = cold('#'); | ||
const e1subs = '(^!)'; | ||
const expected = '#'; | ||
|
||
expectObservable(e1.skipLast(42)).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
|
||
it('should throw if total is less than zero', () => { | ||
expect(() => { Observable.range(0, 10).skipLast(-1); }) | ||
.to.throw(Rx.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 | ||
.mergeMap((x: string) => Observable.of(x)) | ||
.skipLast(42) | ||
.mergeMap((x: string) => Observable.of(x)); | ||
|
||
expectObservable(result, unsub).toBe(expected); | ||
expectSubscriptions(e1.subscriptions).toBe(e1subs); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { Observable } from '../../Observable'; | ||
import { skipLast } from '../../operator/skipLast'; | ||
|
||
Observable.prototype.skipLast = skipLast; | ||
|
||
declare module '../../Observable' { | ||
interface Observable<T> { | ||
skipLast: typeof skipLast; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { Operator } from '../Operator'; | ||
import { Subscriber } from '../Subscriber'; | ||
import { ArgumentOutOfRangeError } from '../util/ArgumentOutOfRangeError'; | ||
import { Observable } from '../Observable'; | ||
import { TeardownLogic } from '../Subscription'; | ||
|
||
/** | ||
* Skip the last `count` values emitted by the source Observable. | ||
* | ||
* <img src="./img/skipLast.png" width="100%"> | ||
* | ||
* `skipLast` returns an Observable that accumulates a queue with a length | ||
* enough to store the first `count` values. As more values are received, | ||
* values are taken from the front of the queue and produced on the result | ||
* sequence. This causes values to be delayed. | ||
* | ||
* @example <caption>Skip the last 2 values of an Observable with many values</caption> | ||
* var many = Rx.Observable.range(1, 5); | ||
* var skipLastTwo = many.skipLast(2); | ||
* skipLastTwo.subscribe(x => console.log(x)); | ||
* | ||
* // Results in: | ||
* // 1 2 3 | ||
* | ||
* @see {@link skip} | ||
* @see {@link skipUntil} | ||
* @see {@link skipWhile} | ||
* @see {@link take} | ||
* | ||
* @throws {ArgumentOutOfRangeError} When using `skipLast(i)`, it throws | ||
* ArgumentOutOrRangeError if `i < 0`. | ||
* | ||
* @param {number} count Number of elements to skip from the end of the source Observable. | ||
* @returns {Observable<T>} An Observable that skips the last count values | ||
* emitted by the source Observable. | ||
* @method skipLast | ||
* @owner Observable | ||
*/ | ||
export function skipLast<T>(this: Observable<T>, count: number): Observable<T> { | ||
return this.lift(new SkipLastOperator(count)); | ||
} | ||
|
||
class SkipLastOperator<T> implements Operator<T, T> { | ||
constructor(private total: number) { | ||
if (this.total < 0) { | ||
throw new ArgumentOutOfRangeError; | ||
} | ||
} | ||
|
||
call(subscriber: Subscriber<T>, source: any): TeardownLogic { | ||
if (this.total === 0) { | ||
// If we don't want to skip any values then just subscribe | ||
// to Subscriber without any further logic. | ||
return source.subscribe(new Subscriber(subscriber)); | ||
} else { | ||
return source.subscribe(new SkipLastSubscriber(subscriber, this.total)); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* We need this JSDoc comment for affecting ESDoc. | ||
* @ignore | ||
* @extends {Ignored} | ||
*/ | ||
class SkipLastSubscriber<T> extends Subscriber<T> { | ||
private ring: T[] = []; | ||
private count: number = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few things:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Blesh I think we have to keep |
||
|
||
constructor(destination: Subscriber<T>, private total: number) { | ||
super(destination); | ||
} | ||
|
||
protected _next(value: T): void { | ||
const len = this.ring.length; | ||
|
||
if (len < this.total) { | ||
this.ring.push(value); | ||
this.count++; | ||
} else { | ||
const idx = this.count++ % this.total; | ||
const oldValue = this.ring[idx]; | ||
|
||
this.ring[idx] = value; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. be sure if your accessing the same property more than once to pull it out into a |
||
this.destination.next(oldValue); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something really wrong about this marble diagram. The bottom
a
should not be emitted at the same time as the topc
, because whenc
occurs, we don't actually yet know that it's the second-last emission. We can only know that when thee1
completes. So the expected should be(ab|)
when the source completes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As soon as
c
is emitted, it is known thata
should not be skipped because it is not one of the last 2 elements and can be emitted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, funny. I may have misunderstood skipLast. Is this how RxJS 4 works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears to be: https://github.com/Reactive-Extensions/RxJS/blob/master/tests/observable/skiplast.js#L248 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this looks correct. if the
skipLast(n)
buffer grows ton + 1
, we know we can drain ton
and emit.