Skip to content
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

Missing doOn* operators #476

Closed
luisgabriel opened this issue Oct 7, 2015 · 9 comments · Fixed by #477
Closed

Missing doOn* operators #476

luisgabriel opened this issue Oct 7, 2015 · 9 comments · Fixed by #477
Assignees

Comments

@luisgabriel
Copy link
Contributor

It seems that doOnNext, doOnError and doOnCompleted are not implemented here yet.

Is it intentional or should we add them?

@staltz
Copy link
Member

staltz commented Oct 7, 2015

do(onNext, () => {}, () => {})

do(() => {}, onError, () => {})

do(() => {}, () => {}, onCompleted)

@mattpodwysocki
Copy link
Collaborator

@staltz that's not quite right if you look at the implementation: https://github.com/Reactive-Extensions/RxJS/blob/master/src/core/perf/operators/tap.js#L55-L86

Since you're lacking the default behavior of the do semantics as well as lacking the thisArg

@benlesh
Copy link
Member

benlesh commented Oct 7, 2015

@mattpodwysocki I'd personally like to do away with thisArg in the library, since we're only targeting browsers that have .bind() now, and ES7 function bind is coming.

@mattpodwysocki
Copy link
Collaborator

@Blesh that's fine but Function#bind is very slow and not likely to be much faster in the future, whereas the thisArg is rather performant in comparison. And if you indeed care about perf, then I'd keep it

@benlesh
Copy link
Member

benlesh commented Oct 7, 2015

@staltz @luisgabriel ... simpler (and perhaps more explicit) in ES6:

obs.do({ error(err) { /* stuff */ } })

// or

obs.do({ error: (err) => /* stuff */ })

@benlesh
Copy link
Member

benlesh commented Oct 7, 2015

Related, though, we should make sure do handles observers and null/undefined values passed to it.

@IanVS
Copy link

IanVS commented May 27, 2016

The migration guide does not mention the removal of the convenience functions doOnNext, doOnError, and doOnCompleted. Doesn't seem worth creating a new issue, but I thought I'd mention it here because it tripped me up a bit in my migration from 4.

@Karasuni
Copy link

Karasuni commented Apr 7, 2017

The documentation http://reactivex.io/documentation/operators/do.html#collapseRxJS still mentions doOnNext, doOnError and doOnCompleted as part of rx.js.

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants