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

Add start method to Observers #3061

Closed
benlesh opened this issue Nov 9, 2017 · 9 comments
Closed

Add start method to Observers #3061

benlesh opened this issue Nov 9, 2017 · 9 comments
Assignees
Labels
feature PRs and issues for features

Comments

@benlesh
Copy link
Member

benlesh commented Nov 9, 2017

The TC39 spec has a start method to prevent this scenario from being a problem:

const subscription = range(0, 10000000).subscribe({
  next(value) { 
    if (value > 3) {
      // try to unsubscribe here, I dare you.
      subscription.unsubscribe();
    }
  }
});

The solution was to add an optional start method to Observer:

const subscription = range(0, 10000000).subscribe({
  start(subs) { this.subscription = subs; },
  next(value) { 
    if (value > 3) {
      // this.subscription is literally the same reference you'd get back
      // from the subscribe call itself.
      this.subscription.unsubscribe();
    }
  }
});
@benlesh benlesh self-assigned this Nov 9, 2017
@benlesh benlesh added Next Major Version feature PRs and issues for features labels Nov 9, 2017
@benlesh
Copy link
Member Author

benlesh commented Nov 9, 2017

And no, I won't be adding this to the 3-callbacks version upfront, it's sort of to cover an edge case, not sure it needs to have some ergonomic considerations.

@felixfbecker
Copy link
Contributor

Why is the scenario a problem?

@benlesh
Copy link
Member Author

benlesh commented Nov 16, 2017

@felixfbecker if a consumer wishes to cancel a synchronous firehose, they cannot do so easily with the basic observable impelmentation. They could "Rx" around it:

const stop = new Subject();
// a hypothetical synchronous firehose of data.
const firehose = range(0, Number.POSITIVE_INFINITY);

firehose.takeUntil(stop);
  .subscribe({
    next(value) {
      doSomeSideEffect(value);
      if (checkSomeExternalState()) stop.next();
      console.log(value);
    }
  });

... but that's a lot of extra hoops to jump through.

@benlesh
Copy link
Member Author

benlesh commented Nov 16, 2017

Regardless, I think we're going to move away from the start method if we use abortSignal: #3122

@benlesh benlesh closed this as completed Nov 16, 2017
@felixfbecker
Copy link
Contributor

Ah, got it.

@martinsik
Copy link
Contributor

It feel like this could be easily solved by using publish():

const source = range(0, 10000000).publish();

const subscription = source.subscribe({
  next(value) { 
    if (value > 3) {
      // try to unsubscribe here, I dare you.
      subscription.unsubscribe();
    }
  }
});

source.connect();

@benlesh
Copy link
Member Author

benlesh commented Nov 28, 2017

@martinsik this is meant to round out the type on its own and it's coming from the standards proposals in flight.

@benlesh
Copy link
Member Author

benlesh commented Nov 28, 2017

And, of note, I was wrong.. even with abort signal, we'd need start.

@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
feature PRs and issues for features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants