baconjs / bacon.js

Functional reactive programming library for TypeScript and JavaScript
https://baconjs.github.io
MIT License
6.47k stars 330 forks source link

.log() prevents the further subscriptions from being invoked #269

Closed Gruzin closed 10 years ago

Gruzin commented 10 years ago
Bacon.fromArray([1,2,3,4,5]).log().map(function(a){return a*10}).log()

just prints 1 2 3 4 5 <end> Shouldn't .log() just trace the value and let everything else work as if there were no .log() call?

philipnilsson commented 10 years ago

The problem is not log, it's fromArray. This combinator will only send it's values to the first subscription encountered, which is the first log statement. Change it to for instance fromArray(...).delay(0), and you'll see results from both log statements.

On Tue, Oct 22, 2013 at 2:07 PM, Gruzin notifications@github.com wrote:

Bacon.fromArray([1,2,3,4,5]).log().map(function(a){return a*10}).log()

just prints 1 2 3 4 5 Shouldn't .log() just trace the value and let everything else work as if there were no .log() call?

— Reply to this email directly or view it on GitHubhttps://github.com/baconjs/bacon.js/issues/269 .

Gruzin commented 10 years ago

Thanks, got it.

Gruzin commented 10 years ago

Isn't that a (minor) problem with fromArray() anyway?

philipnilsson commented 10 years ago

This is part of the semantics of Bacon. It's an important difference between Bacon and for instance Rx.js.

In Rx.js, fromArray will respond with events 1,2,3... each time you subscribe, and would do what you wanted. However, a stream derived from Rx's fromArray will also respond with 1,2,3... if you subscribe to that stream ten minutes later!

This begs the question, when does the events 1,2,3... happen? In Bacon, this is easy to answer. They happen once, synchronously after the first subscription. In Rx, the answer is whenever a new subscription occurs on a stream dervied with fromArray. The problems of this approach is that events basically happen multiple times. It also makes new subscriptions essentially carry side-effects. In most cases Bacon's approach is preferable.

The solution with delay(0) I gave you changes this meaning for this stream into having the events occur asynchronously after the current executing block, with no delay. This maintains proper semantics, as events still only occur once, but you still have time to create multiple subscriptions before they happen.

On Tue, Oct 22, 2013 at 2:41 PM, Gruzin notifications@github.com wrote:

Isn't that a (minor) problem with fromArray() anyway?

— Reply to this email directly or view it on GitHubhttps://github.com/baconjs/bacon.js/issues/269#issuecomment-26799169 .

raimohanska commented 10 years ago

@philipnilsson that's a very good description of the fromArray situation! We might add this to the FAQ on Wiki, to be maybe later moved to the github.io site.

Gruzin commented 10 years ago

Thanks for your explanations, it kind of makes sense on some level.

But it's still somewhat baffling that there're two kinds of event streams, one that works and the other that doesn't. I mean, once you've set up your network of signals and properties that are based on some input signal stream and then you substitute the input signal with something equivalent(sending the same values), you still expect your network to work the same. However, it turns out it might not because the signals are (surprise!) not equivalent.

I have no experience with Rx(JS) so please forgive me if I'm wrong, but it seems that what you're describing resembles hot and cold observables. fromArray() is like "hot" and fromArray().delay(0) is like "cold".

Actually, I've encountered this issue not with fromArray() but with a bridge to dojo/topic that I had first written rather naively:

    function fromDojoTopic(topic, eventTransformer) {
      return Bacon.fromBinder(function(handler) {
        var handle = dojoTopic.subscribe(topic, handler);
        return function() {
          return handle.remove();
        };
      }, eventTransformer)

then I noticed that when I add logging my program simply stops working. I tried to reproduce the problem with fromArray() just because it's the first thing that comes to mind and the problem happened indeed, which now turns out to be a coincidence - my stream shared this property with fromArray().

So, I think it would be great if that thing with "two kinds of streams" is mentioned in the doc so that people who start working with Bacon.js can be aware of those potential problems. Or there's some guide to those who write their own streams mentioning this issue.

Thanks!

philipnilsson commented 10 years ago

@Gruzin Actually Bacon doesn't really have hot and cold observables as such, though it is the case that subscribing has side-effects the first time you subscribe. Since log subscribes to the stream, it can indeed change the meaning of your program.

I suggest you define

Bacon.Observable.prototype.debug = function() { return this.doAction(function(x) { console.log(x) }) }

and you can do logging without interfering with the meaning of the stream. Of course you must then make sure to subscribe to the debugged stream with e.g. subscribe, onValue or even log.

philipnilsson commented 10 years ago

Just to be clear

var stream  = Bacon.interval(100).map('hi!');

// does nothing. unlike log, debug will not 'pull' values, which is exactly
// what makes it appropriate for your use, as it won't affect the debugged stream
stream.debug();

// will do logging. this is because onValue will 'pull' the values
stream.debug().onValue(...) 

// will also do the same logging. (it'll log 'hi').
// You only need onValue somewhere further down the "chain"
stream.debug().map('mapped').filter(somePredicate).onValue(...) 
raimohanska commented 10 years ago

Btw should we include debug in Bacon? For me it would make sense and would not bloat the lib too much.

Regarding hot/cold, I don't consider fromArray() based streams to be proper streams exactly because of their nasty habit of spitting contents out to the first subscriber. So, you should avoid exposing these streams to multiple subscribers.

On the other hand, Bacon.once() and its friends are very useful in combination with flatMap, where they don't cause any problems, because they aren't exposed as top-level streams and thus always have just one subscriber, i.e. the flatmap mechanism itself.

Gruzin commented 10 years ago

Thanks for the debug solution.

I would also suggest it would be nice to have a global flag to turn debugging on/off so that you don't have to change all of your code to debug it. Not sure how a global flag agrees with the functional nature of the library though...

Gruzin commented 10 years ago

Or maybe not even a flag but you can say like: stream1.debug("a/b/c")... stream1.debug("a/b/d")... stream1.debug("e/f")... and then say like Bacon.debugPattern = "a/*" etc.

raimohanska commented 10 years ago

Not going to turn Bacon.js into Log4js :)

I suggest you implement your solution like @philipnilsson suggested, so that you get the flexibility you want.

Gruzin commented 10 years ago

Yes, that solution is fine, thank you.

philipnilsson commented 10 years ago

If that's what you want you can of course simply modify the code above

window.debugPattern = 'a/';
Bacon.Observable.prototype.debug = function(pattern) {
  return this.doAction(function() { if (pattern.indexOf(window.debugPattern) == 0) console.log(x) }) 
}

This is too specific for general inclusion in Bacon though.

Gruzin commented 10 years ago

Well, I just mean that, I think, a .log() or .debug() method should have some means to suppress debug output without having to change the code. Maybe it can just accept a function that will do the logging.

raimohanska commented 10 years ago

Yeah, if you intend to add the logging stuff to your application. Still suggest to roll your own bacon.logging lib. Then share it with the rest of us :)

I've used .log() just when debugging/developing, then removed the calls before commit.