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

.onValue side-effect function can unsubscribe itself by returning Bacon.noMore #734

Open
semmel opened this issue Mar 20, 2019 · 1 comment

Comments

@semmel
Copy link
Member

semmel commented Mar 20, 2019

My superficial issue is to document explicitly that the side-effect f in observable.onValue(f) like observable.subscribe(f) has the ability to cancel its subscription of stream updates by returning Bacon.noMore.

After hours of debugging (see code example below) this really came as a surprise for me because I've never had an event consumer function to update the event flow logic. Up to now I did not care what my side-effects return: Promises, Strings or whatever...

Is this an API which is worth supporting? I mean:

  • Does this encourage good programming practise?
  • Can this break for innocent-looking side-effect functions? (E.g. returning a WebComponent tag which could be <no-more>, or the example below ?)

Breaking a stable API is bad but IMO this feature is dangerous.

What does everybody think?

Here my example of two nested Bacon.fromBinders. Can you spot why outerStream is unsubscribed even though nobody calls unsubscribe or returns Bacon.noMore explicitly?

var outerStream = Bacon.fromBinder(function myOuterBinder(sinkOutside) {
    const innerStream = Bacon.fromBinder(function myInnerBinder(sinkInside) {
	 	// quasi sinking sinkInside into sinkOutside
	 	setTimeout(sinkOutside, 2000, sinkInside);
		return () => console.log("cleaning inner"); 
	});
	innerStream.take(1).onValue(x => console.log("inner consumer", x));
	return () => console.log("outer clean up");
});
outerStream.onValue(f => f("inner value")); // f is sinkInside actually
/* The output is:
cleaning inner <-- OK because take(1)
inner consumer inner value <-- OK
outer clean up  <--- THIS WAS A SURPISE! 
*/

The solution begins with that sinkInside of the inner stream's binder returns Bacon.noMore because of take(1). This is then fed to the outer stream's .onValue() side-effect invoker by the ES6 arrow => function in the last line.

I could blame it on ES6 arrow functions because re-writing the side-effect to a "regular" function expression fixes the behaviour.

// ...
outer.onValue(function(f){ f("inner value"); });
/*
cleaning inner
inner consumer inner value
*/

However, if someone comes along and refactors it back into an arrow function the bug reappears!

@raimohanska
Copy link
Contributor

I've never used Bacon.noMore in a subscriber either. I think I've heard people using it somewhere though. I could imagine it being convenient for a "do this side effect and ignore future events if something specific happened while performing the side effect" case but personally I cannot recall ever feeling like doing that.

Would be great to hear if someone needs this feature. But seems like there's not so much interest in a conversation here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants