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

feat(tap): Adds subscribe, unsubscribe, finalize handlers #6527

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Jul 16, 2021

This adds a common request/task for RxJS users, which are three new handlers:

  • subscribe: fires on subscription to the source
  • unsubscribe: fires when the subscription to the result is unsubscribed from, but NOT if the source completes or errors
  • finalize: always fires on finalization, (basically equivalent to finalize)

This is useful for situations where you'd like to know that your consumer has decided to unsubscribe (and you're not terminating because of a complete or an error).

let subject = new Subject();

const sub1 = subject.pipe(
   tap({ 
      subscribe: () => console.log('subscribed'),
      complete: () => console.log('complete'),
      unsubscribe: () => console.log('unsubbed'),
      finalize: () => console.log('finalize')
   })
).subscribe();
// logs "subscribed"

sub1.unsubscribe();
// logs "unsubbed"
// logs "finalized"

const sub1 = subject.pipe(
   tap({ 
      complete: () => console.log('complete'),
      unsubscribe: () => console.log('unsubbed')
   })
).subscribe();
// logs "subscribed"

subject.complete();
// logs "complete"
// logs "finalized"

See included tests for more information.

This adds a common request/task for RxJS users, which are three new handlers:

- `subscribe`: fires on subscription to the source
- `unsubscribe`: fires when the subscription to the result is unsubscribed from, but NOT if the source completes or errors
- `finalize`: always fires on finalization, (basically equivalent to `finalize`)
@benlesh benlesh requested a review from cartant July 16, 2021 19:32
@benlesh benlesh added the 7.x Issues and PRs for version 7.x label Jul 16, 2021
Copy link
Contributor

@backbone87 backbone87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reasoning behind doing this only for the tap operator and not for Subscribable.subscribe?

isFunction(observerOrNext) || error || complete
? // tslint:disable-next-line: no-object-literal-type-assertion
({ next: observerOrNext as Exclude<typeof observerOrNext, Partial<TapObserver<T>>>, error, complete } as Partial<TapObserver<T>>)
: observerOrNext;

// TODO: Use `operate` function once this PR lands: https://github.com/ReactiveX/rxjs/pull/5742
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think this comment can be removed

@kwonoj
Copy link
Member

kwonoj commented Jul 20, 2021

Just checking: did we discuss this in core meeting?

@benlesh
Copy link
Member Author

benlesh commented Jul 27, 2021

@backbone87 the main reasoning is we're not attempting to change the semantics of observable, (which Subscribable is really just a super type of), this is about side-effects and observation on the part of the producer, which tap is defining.

import { interval, share, tap, take } from 'rxjs';

const producer = interval(1000).pipe(
  share(),
  take(100),
  tap({ 
    unsubscribe: () => console.log('a user has unsubscribed'),
  })
);

// At this point `producer` could be handed off to anyone.
// `tap` is concerned with side effects that _producer_ needs. Not the consumer.


// Here we have a consumer
const subscription = producer.subscribe({
   next: console.log,
   // (hypothetical)
   unsubscribe: () => console.log('you unsubscribed yourself')
});

// Why didn't you just console log here? :shrug:
subscription.unsubscribe(); 

the subscribe handler on the consumer end is honestly just as silly:

producer.subscribe({
  // Hypothetical:
  subscribe: () => console.log('you subscribed, lol');
});

Finalize might have merit however, and that's possibly worth discussing. Interestingly, it can be handled now like so (it's just non-obvious):

const subscription = producer.subscribe(console.log);

subscription.add(() => console.log('finalized');

In the future if we're using AbortSignal, and we land on not minding async completion (That's probably a non-starter), we could leverage forEach and Promise.prototype.finally:

// Hypothetical
const controller = new AbortControlelr();
const { signal } = controller;
producer.forEach(console.log, { signal }).finally(() => console.log('finalized'))

You can obviously do this now with forEach, it's just not cancellable. :)

@benlesh
Copy link
Member Author

benlesh commented Jul 27, 2021

@kwonoj

Just checking: did we discuss this in core meeting?

Yes, but it's been a long time since we did.

@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x Issues and PRs for version 7.x AGENDA ITEM Flagged for discussion at core team meetings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants