gmac / backbone.epoxy

Declarative data binding and computed models for Backbone
http://epoxyjs.org
MIT License
615 stars 89 forks source link

handle a race between reset() and in-flight events #78

Closed disruptek closed 9 years ago

disruptek commented 10 years ago

This race condition can occur when a change to a bound attribute results in the binding being removed before the binding's reset() is invoked.

gmac commented 10 years ago

Okay... I definitely don't question the validity of this report; it sounds like a very viable issue. All the same, could you please provide a practical example of the issue? I'd just like to see the full issue in action before approving a fix.

disruptek commented 10 years ago

I'm sorry about the delay, but apparently I don't have time to compose a failing test. It's a pretty straightforward failure scenario though; I believe I encountered it when a bound value change resulted in the View closing...

samccone commented 10 years ago

Hmm now I am curious,

so an event is emitted via a

As i understand the JS stack this is how this would behave

a =====> receiver
      reset===> receiver

now.. the only way for a to arrive before reset would be the use of a requestAnimationFrame or setTimeout

but perhaps I am missing something.

Would love some insight and knowledge drop @disruptek

disruptek commented 10 years ago

I don't follow your "JS stack" example, but rather than you attempt to explain it to me, let me just try a bit harder to explain this PR to you. :smile:

I believe that the following scenario serves to illustrate the problem, but again, I don't have a demo. I've been using this code for months now and I don't really fancy reverting and then finding the failures and shrinking them to the smallest possible test case.

Anyway, in the example scenario, you have a view that takes login credentials from the user and displays a bound value -- the state of connectivity to a server. This state may vary from several values; let's say it starts with Offline, proceeds to Connecting, and perhaps finally reaches Online. You have a trigger on the change of this value such that the view is closed when the state changes to Online. When the state changes to Online and closes the view before the binding is propagated, an error will be raised in Epoxy due to the $el no longer existing.

IIRC, this is merely a byproduct of the way Backbone's crude event system works -- and more specifically, the way Underscore's each iterates over a copy of an array as opposed to operating on that array in place. As a result, an event handler which removes other event handlers sharing the same trigger is unable to prevent them from executing.

Sure, there are other ways around the problem, and if it were my code, perhaps I'd write a much more involved solution or simply change the design to address the problem more formally. But as a one-liner to gracefully handle the issue without stepping on any other toes in Epoxy or potentially conflicting with future changes to Backbone's events, I think it's pretty reasonable.

samccone commented 10 years ago

:+1: thanks for the great explanation @disruptek I see your point thanks!

gmac commented 9 years ago

Sounds good, thanks for the fix. Unfortunately, I just changed whitespace and made a right mess of merges... bad planning. Your fix is in there (see ce50c0e), although I'm sorry you don't get the proper commit attribution.