feat(operator): add skipWhile operator #703
Conversation
828ae99 to
ac093f7
Compare
| var expected = '----------------|'; | ||
|
|
||
| expectObservable(source.skipWhile(function () { return {}; })).toBe(expected); | ||
| }); |
There was a problem hiding this comment.
Can you add a test case where the predicate throws an error?
|
In RxJS4, the predicate function takes as third argument (after the value and index) the observable object being subscribed... I'm not sure what is the purpose of this and could not find any... Do you think we need to keep it ? |
| } | ||
|
|
||
| _next(value: T): void { | ||
| const destination = this.destination; |
There was a problem hiding this comment.
Like for everything else, you could store const skipping = this.skipping since it's used multiple times.
There was a problem hiding this comment.
Not really because it is mutated on line 40.
|
I think PR's OK. @jinroh , you're fast! :) |
I believe we kept it in |
| import {errorObject} from '../util/errorObject'; | ||
| import {bindCallback} from '../util/bindCallback'; | ||
|
|
||
| export function skipWhile<T>(predicate: (x: T, index: number) => boolean, thisArg?: any) { |
There was a problem hiding this comment.
This should define a return value of : Observable<T>
|
Actually, there are mixture currently, refer |
|
Oops didn't mean to close. Fat fingers. Anyhow...
That's fine. It really doesn't make much sense anyhow. |
: +1 too. Maybe can discuss later to find if anyone has usecase. |
|
I think most of issues in this PR's now settled down, will merge in around hr. (having around hr of buffer just in case if @Blesh might have some additional suggestion) |
|
@kwonoj 👍 LGTM |
(#696)
Waiting for your reviews !