Switch to using $applyAsync instead of checking for digest cycles and using direct $apply

[lauripiispanen]

Use of $apply is discouraged, but this will cause stream digests to always be evaluated asynchronously. Test cases will be affected, but must also try to check if this has other implications to existing code.

[codeflows]

I just started using angular-bacon in a directive with isolate scope. I'm currently getting the error [$rootScope:inprog] $digest already in progress because $apply is being called on the isolate scope when its parent scope's phase is "$digest".

On a quick look changing the if at https://github.com/lauripiispanen/angular-bacon/blob/master/src/angular-bacon.coffee#L31 to something like if (!$scope.$$phase && (!$scope.$parent || !$scope.$parent.$$phase)) { ... } seems to fix this but is naturally more hackish than we'd like ;)

Maybe using $evalAsync would also fix this situation?

I would take a stab at this, but unfortunately don't have time at the moment :(

[ekarlsso]

First, thanks for the nice library! It has been helping when playing with angular + bacon combo. Now when trying out the "componitization of angular" as described in http://teropa.info/blog/2014/10/24/how-ive-improved-my-angular-apps-by-banning-ng-controller.html you get bitten by this issue as @codeflows mentioned.

About the differences between $applyAsync and $evalAsync. Please correct if I'm wrong but my understanding is that the: 1) evalAsync schedules operation that (A) - if there is ongoing digest - is executed still in the same digest but just later in the loop OR (B) if there is no ongoing digest a timeout is set for later operation 2) applyAsync always schedules a timer (about 10 ms)

Based on this I think the evalAsync would be better option here (?) - you could avoid unnecessary timeouts.

Was the comment about the test cases mostly related to this library's own test cases - or concern related to other project's tests using this library?

[lauripiispanen]

You're totally right! In this scenario $evalAsync would actually be preferred, since we don't want an extraneous digest cycle if one is not needed. A timeout would also be an unnecessary hit on performance, since it would cause browser repaints if DOM was modified etc.

As for the test cases comment, that was mostly in relation to the this project's test cases. Switching to $evalAsync will break any existing code that relies on synchronous digestion of streams. However, my take on this is that relying on such implementation details in FRP is an anti-pattern, so I would be inclined to deem it acceptable for such code to break. Just to be safe, though, I've incremented major version number to mark this as a potentially backwards-incompatible change.

There's now a branch with $evalAsyncinstead of apply. I'd love it if you could test it in your projects as well. I did try it with the example provided, and it works fine.

[ekarlsso]

Wow! that was quick. I try to have a look at this during the next days. We would definitely benefit from this :smile:

[ekarlsso]

Took a bit more time. I played now with this one and it looks like that it works fine. I vote for release :)

[lauripiispanen]

Released! Thank you for the feedback!