Fix infinite loop when sorting from reaction function

dart-archive / observe

Support for marking objects as observable, and getting notifications when those objects are mutated

https://pub.dartlang.org/packages/observe

BSD 3-Clause "New" or "Revised" License

13 stars 6 forks source link

Fix infinite loop when sorting from reaction function #80

Closed eRaz0rHead closed 8 years ago

eRaz0rHead commented 8 years ago

Fixes https://github.com/dart-lang/observe/issues/79

googlebot commented 8 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

eRaz0rHead commented 8 years ago

I signed it!

googlebot commented 8 years ago

CLAs look good, thanks!

jmesserly commented 8 years ago

A few suggestions for the test, otherwise looks good! (and I'm happy to merge & then fix it up as well, if that works for you)

eRaz0rHead commented 8 years ago

I believe we can use the standard future, rather than the delay. The key to reproducing the bug is that there is still a listener when the reSort happens. Since that's in the microTask loop, delaying clean up with a simple future should still work.
Or be lazy and leave the subscription active until the test exits .. that also works

eRaz0rHead commented 8 years ago

Updated the test to use the synchronous deliverListChanges as suggested.

jmesserly commented 8 years ago

LGTM, thanks!