split the method “subscribe” into two steps to get the subscription instance in handle function

[ArthasModern]

Description:

Related issue (if exists):

[rxjs-bot]

	Warnings
⚠️	commit message does not follows conventional change log (1)

	Messages
📖

(1) : RxJS uses conventional change log to generate changelog automatically. It seems some of commit messages are not following those, please check contributing guideline and update commit messages.

CJS: 1385.4KB, global: 753.9KB (gzipped: 120.8KB), min: 145.5KB (gzipped: 31.4KB)

Generated by 🚫 dangerJS

[coveralls]

Coverage decreased (-0.07%) to 97.012% when pulling 2ad5036 on ArthasModern:master into c1721e3 on ReactiveX:master.

[ArthasModern]

Fuck, too complicated to commit!

[Brooooooklyn]

may I ask what's the intent of split subscribe ?

[ArthasModern]

Hi @Brooooooklyn
split the method “subscribe” into two steps
for this case: we need to get the subscription instance in the handle function

@example unsubscribe in the handle function

const subscription = Rx.Observable.of(1, 2, 3).subscribe(
function(value) {
// do handle here, then we need unsubscribe immediately!
subscription.unsubscribe();
// error: subscription undefined!!!
},
undefined,
undefined
);

[ArthasModern]

@Brooooooklyn because the subscribe function do not return the subscription immediately, sometimes we need to get the subscription instance before the action start.

[ArthasModern]

@Brooooooklyn do you agree with me？and you know how to submit it correctly？

[paulpdaniels]

@ArthasModern Thanks for the PR! However, IMO I don't think this is a good way to approach this problem. Explicit unsubscribe, especially in the subscribe block is an anti-pattern. If you encounter this then more than likely you want to be using something like takeWhile instead. @benlesh wrote a good article on this as well: https://medium.com/@benlesh/rxjs-dont-unsubscribe-6753ed4fda87.

I'll wait for some of the core contributors to chime in, since I am pretty far removed from what does or doesn't go into this library, but that is my two cents.

[Brooooooklyn]

I agree with @paulpdaniels , you should never unsubscribe you subscription in subscriber

[ArthasModern]

@paulpdaniels Thank you very much for your answer! take takeWhile takeUntil etc. is very useful! Combined with filter , there are many ways to solve my problem. I just think it's not convenient enough! and there is no harm to split subscribe as an extension.

[ArthasModern]

OK, it's just my personal opinion. Thank you very much for your advice!
@Brooooooklyn @paulpdaniels

[cartant]

See #3061 and #3122 re: unsubscribing in subscribe.

[kwonoj]

As suffeciently described above there are discussions of controlling interfaces for observables and I don't think we'll pursue this proposed interfaces.

[benlesh]

I don't think we can accept this PR. There are other designs around how to handle this.

One is the start event, added to the TC39 Proposal, and it gives you the Subscription just before the Observable body is executed. The other is to hand the Subscription to the next callback as a second argument, and that's being looked at in the WHATWG proposal. Both proposals are cooperating and will align at some point, but it's not a big issue. So until the proposals settle, we're going to hold off on adding anything. (I had added the start handler briefly in the past).

Thank you so much for your thought and effort, @ArthasModern! I know it's hard to have a PR closed. Hopefully you continue to contribute!

[lock]

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.