Property not up to date when combined and sampled

[niklasvh]

I came across an issue where the property is not up to date to the value it should be, and narrowed it down to when using it with combine and sampledBy.

A simplified case of it can be seen at http://jsfiddle.net/76HxL/
The issue is resolved if the combinedStream.onValue(function() { }); subscriber is removed (http://jsfiddle.net/76HxL/1/), but that isn't really an option (the example is just a very simplified case).

Is this an expected behavior?

[raimohanska]

Nope, that's some nasty behavior indeed.

I tidied up the fiddle a bit: http://jsfiddle.net/DPHBB/

[raimohanska]

Next up: write a failing test into the Bacon.js test suite. Wanna try?

[raimohanska]

My quick analysis is that this is related to the non-atomicity of Property updates, that's already coverd in #85.

In any case, a failing unit test is the way to go from here.

[niklasvh]

Added a few tests so far, didn't get a chance to test it with the atomic branch yet though.

What's currently the status with the branch?

[raimohanska]

The branch contains the first shot at atomic upds. Not updated.

-juha-

On 29.4.2013, at 22.50, Niklas von Hertzen [email protected] wrote:

Added a few tests so far, didn't get a chance to test it with the atomic branch yet though.

What's currently the status with the branch?

—
Reply to this email directly or view it on GitHub.

[raimohanska]

Tried your tests on the atomic-updates branch. Fails similarly to master.

[raimohanska]

Ok then. Re-implementated the quite-crufty sampledBy with a simpler one. Now in the atomic-updates branch, your tests pass. One more reason to get that shit into master, right?

[niklasvh]

Nice, I'll give the branch a go

[raimohanska]

@niklasvh Atomic Updates now included in master and in release 0.4.0. Would you like to try if they solve your problem?

[niklasvh]

Awesome! I'll give them a go tomorrow and let you know if there are still any issues

[niklasvh]

This fixed the issue and a number of other related ones. I came across a similar bug yesterday which this update did not fix though, but I'll open another issue for it once I've managed to narrow the problem down into a simplified case.