From 6dbb6d598525e1f4697be922871e2c97e230e32b Mon Sep 17 00:00:00 2001
From: Chris Spencer <chris.ag.spencer@googlemail.com>
Date: Sat, 15 Aug 2015 17:33:52 +0100
Subject: [PATCH] Adjust notifyObservers iteration when unwatch function is
 called during iteration [Fixes #151]

---
 src/change-observer.js         | 16 ++++++++++++++--
 tests/change-observer-tests.js | 11 +++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/change-observer.js b/src/change-observer.js
index 5f1ce84..7150f94 100644
--- a/src/change-observer.js
+++ b/src/change-observer.js
@@ -18,6 +18,7 @@ class ChangeObserver {
     this.__evaluator = evaluator
     this.__prevValues = Immutable.Map()
     this.__observers = []
+    this.__notifyIndex = -1
   }
 
   /**
@@ -27,7 +28,13 @@ class ChangeObserver {
     if (this.__observers.length > 0) {
       var currentValues = Immutable.Map()
 
-      this.__observers.forEach(entry => {
+      // The unwatch function needs to read and modify where in the
+      // iteration we are, so that removing the current or previous
+      // observers does not cause us to skip observers.
+      for (this.__notifyIndex = 0;
+           this.__notifyIndex < this.__observers.length;
+           ++this.__notifyIndex) {
+        var entry = this.__observers[this.__notifyIndex]
         var getter = entry.getter
         var code = hashCode(getter)
         var prevState = this.__prevState
@@ -46,8 +53,9 @@ class ChangeObserver {
           entry.handler.call(null, currValue)
           currentValues = currentValues.set(code, currValue)
         }
-      })
+      }
 
+      this.__notifyIndex = -1
       this.__prevValues = currentValues
     }
     this.__prevState = newState
@@ -73,6 +81,10 @@ class ChangeObserver {
       var ind = this.__observers.indexOf(entry)
       if (ind > -1) {
         this.__observers.splice(ind, 1)
+        // If we are at or before the current notifyIndex, decrement it.
+        if (ind <= this.__notifyIndex) {
+          this.__notifyIndex -= 1
+        }
       }
     }
   }
diff --git a/tests/change-observer-tests.js b/tests/change-observer-tests.js
index 99f24ff..4b49360 100644
--- a/tests/change-observer-tests.js
+++ b/tests/change-observer-tests.js
@@ -78,6 +78,17 @@ describe('ChangeObserver', () => {
         expect(mockFn2.calls.count()).toBe(1)
       })
     })
+
+    it('should not skip observers when handler causes unobserve', () => {
+      var getter = ['foo', 'bar']
+      var mockFn = jasmine.createSpy()
+      var unreg = observer.onChange(getter, () => unreg())
+      observer.onChange(getter, mockFn)
+
+      observer.notifyObservers(initialState.updateIn(getter, x => 2))
+
+      expect(mockFn.calls.count()).toBe(1)
+    })
   })
   // TODO: test the prevValues and registering an observable
 })