skipDuplicates with Property and take(1)

[bogus34]

Test example:

---

B = require 'baconjs'

b = new B.Bus() p = b.toProperty() p.onValue -> # force property update s = p.skipDuplicates()

b.push 'foo'

s.take(1).onValue console.log, '1' s.take(1).onValue console.log, '2' s.take(1).onValue console.log, '3'

---

Expected output: 1 foo 2 foo 3 foo

Actual output: 1 foo 3 foo

Why is it behaves so strange?

[bogus34]

Btw, if I write p = b.toProperty().skipDuplicates() without intermediate variable, it works correctly

[philipnilsson]

I can repro, that's really weird, and kinda unsettling. I might take a look at this. @raimohanska , any ideas?

[raimohanska]

I'm surprised. Did you already make a failing test for this?

[raimohanska]

I found it. The problem is in skipDuplicates implementation where it keeps track of the current value the wrong way. It has a value in a global scope, meaning that when all subscribers are disposed, the value is still there. When the next subscriber comes in, the old current value is applied and the new value is possibly discarded even when it shouldn't.

I've got a fix too, and I'll do some more checks to make sure it works correctly in all cases.

[raimohanska]

Oh, actually my initial fix broke EventStream.skipDuplicates in a very special case. The correct fix is simpler: never drop an Initial event in skipDuplicates. That's it.

[raimohanska]

Fixed in 0.6.3

[bogus34]

Actually, I've used my own version of skipDuplicates that never drops initial events as a workaround. Sorry not mention it here. But I think this is only a workaround because it leaves some open questions.

1)

p = b.toProperty() s = p.skipDuplicates()

seems to be an equivalent to p = b.toProperty().skipDuplicates() s = p

but it isn't

2) if I rewrote initial code as

b = new B.Bus() p = b.toProperty()

p.onValue -> # force property update

s = p.skipDuplicates() s.subscribe (e) -> console.log '-', e.isInitial(), e.value() # mark 1 b.push 'foo' s.take(1).onValue console.log, '1' s.take(1).onValue console.log, '2' s.take(1).onValue console.log, '3'

I'll see that marked subscribe never gets an initial event but later does and it doesn't loose events anymore.

3) if I add more subscribes like "s.take(1).onValue console.log, 'N'" to first version of test code, I'll see that only second one are loosed. 3, 4, 5 and so on are printed and all theese events are Initial.

All tested with version 0.6.2

[raimohanska]

Did you try 0.6.3? Did it work for you?

[bogus34]

It works. Just mentioning some points, that looks unclear for me. Thank you.

[raimohanska]

The weird thing there was that

• 1 works
• 2 doesn't work
• 3 works

This was caused by the following

• 1 works because it's the first one, so there's no duplicity to be skipped. The take(1) part causes the subscription to be disposed, so after this, s has no subscribers. However, the internal status of skipDuplicates is such that it now remembers that "foo" is the previous value
• 2 fails to produce a value because the Initial event contains the same value "foo" as is currently stored by skipDuplicates. This subscription is not disposed by take(1) because no value was producted. Thus s has a subscriber now.
• 3 works because the internal PropertyDispatcher of s bounces the current value, instead of re-subscribing to the underlying property p. This happens always when a Property already has subscribers.

After my fix, skipDuplicates always passes through Initial events and should work just fine.

[bogus34]

Thanks for detailed answer and for this great library!