Merge ade6600 into 03b4743

src/change-observer.js

@@ -1,6 +1,9 @@
 var Immutable = require('immutable')
+var KeyPath = require('./key-path')
+var Getter = require('./getter')
 var hashCode = require('./hash-code')
 var isEqual = require('./is-equal')
+var each = require('./utils').each
 
 /**
  * ChangeObserver is an object that contains a set of subscriptions
@@ -61,20 +64,69 @@ class ChangeObserver {
    * @return {function} unwatch function
    */
   onChange(getter, handler) {
+    var entry
+    var unwatch = () => {
+      // TODO: untrack from change emitter
+      var ind = this.__observers.indexOf(entry)
+      if (ind > -1) {
+        this.__observers.splice(ind, 1)
+      }
+    }
+
     // TODO: make observers a map of <Getter> => { handlers }
-    var entry = {
+    entry = {
       getter: getter,
       handler: handler,
+      unwatchFn: unwatch,
     }
+
     this.__observers.push(entry)
+
     // return unwatch function
-    return () => {
-      // TODO: untrack from change emitter
-      var ind = this.__observers.indexOf(entry)
-      if (ind > -1) {
-        this.__observers.splice(ind, 1)
+    return unwatch
+  }
+
+  /**
+   * Calls the unwatchFn for each change observer associated with the passed in
+   * keypath or getter. If a handler function is passed as well, then the
+   * unwatchFn is called only for the change observer that holds that handler.
+   *
+   * UnwatchFns are called up the dependency chain.
+   *
+   * @param {KeyPath|Getter} getter
+   * @param {function} handler
+   */
+  unwatch(getter, handler) {
+
+    // Returns unwatchFn if no handler, or if handler is the observer's handler
+    // Otherwise returns `null`
+    var unwatchIfShould = entry => {
+      if (!handler) {
+        return entry.unwatchFn
+      }
+
+      if (handler === entry.handler) {
+        return entry.unwatchFn
       }
+
+      return null
     }
+
+    var isKeyPath = KeyPath.isKeyPath(getter)
+
+    // Collects all the unwatchFns that need to be called, without invoking
+    // them, so as to not mutate the observers collection, then invokes each.
+    each(this.__observers.map(entry => {
+      if (isKeyPath && Getter.wasKeyPath(entry.getter)) {
+        if (KeyPath.same(getter, entry.getter[0]) || KeyPath.isUpstream(getter, entry.getter[0])) {
+          return unwatchIfShould(entry)
+        }
+      }
+
+      if (entry.getter === getter) {
+        return unwatchIfShould(entry)
+      }
+    }), unwatchFn => unwatchFn && unwatchFn())
   }
 
   /**

src/getter.js

@@ -48,10 +48,21 @@ function fromKeyPath(keyPath) {
   return [keyPath, identity]
 }
 
+/**
+ * Determines if a getter was converted from a plain KeyPath, ex. ['a', 'b'],
+ * using Getter.fromKeyPath
+ * @param {Getter}
+ * @return {boolean}
+ */
+function wasKeyPath(getter) {
+  return getComputeFn(getter) === identity
+}
+
 
 module.exports = {
   isGetter: isGetter,
   getComputeFn: getComputeFn,
   getDeps: getDeps,
   fromKeyPath: fromKeyPath,
+  wasKeyPath: wasKeyPath,
 }

src/key-path.js

@@ -6,9 +6,71 @@ var isFunction = require('./utils').isFunction
  * @param {*} toTest
  * @return {boolean}
  */
-exports.isKeyPath = function(toTest) {
+var isKeyPath = exports.isKeyPath = function isKeyPath(toTest) {
   return (
     isArray(toTest) &&
     !isFunction(toTest[toTest.length - 1])
   )
 }
+
+/**
+ * Determines if two keyPaths reference the same key
+ * @param {array} keyPath
+ * @param {array} otherKeyPath
+ * @return {boolean}
+ * @example
+ * same(['some', 'keypath'], ['some', 'keypath']) // => true
+ * same(['some', 'keypath'], ['another', 'keypath']) // => false
+ */
+exports.same = function same(keyPath, otherKeyPath) {
+  if (!isKeyPath(keyPath) || !isKeyPath(otherKeyPath)) {
+    return false
+  }
+
+  var len
+  var i = len = keyPath.length
+
+  if (len !== otherKeyPath.length) {
+    return false
+  }
+
+  while (i--) {
+    if (keyPath[i] !== otherKeyPath[i]) {
+      return false
+    }
+  }
+
+  return true
+}
+
+/**
+ * Determines if a keyPath references a key that is upstream to another keyPath
+ * @param {array} keyPath
+ * @param {array} downstreamKeyPath
+ * @return {boolean}
+ * @example
+ * isUpstream(['some', 'keypath'], ['some', 'keypath', 'with depth']) // => true
+ * isUpstream(['some', 'keypath'], ['some', 'other', 'keypath']) // => false
+ */
+exports.isUpstream = function isUpstream(keyPath, downstreamKeyPath) {
+  if (!isKeyPath(keyPath) || !isKeyPath(downstreamKeyPath)) {
+    return false
+  }
+
+  var i = 0
+  var len = keyPath.length
+
+  if (len >= downstreamKeyPath.length) {
+    return false
+  }
+
+  while (i < len) {
+    if (keyPath[i] !== downstreamKeyPath[i]) {
+      return false
+    }
+
+    i++
+  }
+
+  return true
+}

src/reactor.js

@@ -101,6 +101,20 @@ class Reactor {
     return this.__changeObserver.onChange(getter, handler)
   }
 
+  /**
+   * Removes a change observer that has previously been set
+   *
+   * 1. unobserve(keyPath) Removes all observables for keyPath
+   * 2. unobserve(getter) Removes all observables for getter and getter dependencies
+   * 3. unobserve(keyPath|getter, handlerFn) Removes the specific handlerFn for
+   *    the passed in keyPath or getter.
+   *
+   * @param {KeyPath|Getter} getter
+   * @param {function} handler
+   */
+  unobserve(getter, handler) {
+    this.__changeObserver.unwatch(getter, handler)
+  }
 
   /**
    * Dispatches a single message

tests/change-observer-tests.js

@@ -78,6 +78,123 @@ describe('ChangeObserver', () => {
         expect(mockFn2.calls.count()).toBe(1)
       })
     })
+
+    describe('unregistering change handlers', () => {
+      it('should de-register by KeyPath', () => {
+        var mockFn = jasmine.createSpy()
+
+        observer.onChange(Getter.fromKeyPath(['foo']), mockFn)
+        observer.unwatch(['foo'])
+        observer.notifyObservers(initialState.updateIn(['foo', 'bar'], x => 2))
+
+        expect(mockFn.calls.count()).toBe(0)
+      })
+
+      describe('when a deep KeyPath is registered', () => {
+        it('should de-register the entire KeyPath', () => {
+          var mockFn = jasmine.createSpy()
+
+          observer.onChange(Getter.fromKeyPath(['foo', 'bar']), mockFn)
+          observer.unwatch(['foo', 'bar'])
+
+          observer.notifyObservers(initialState.updateIn(['foo', 'bar'], x => {
+            return Map({
+              'baz': 2,
+            })
+          }))
+
+          expect(mockFn.calls.count()).toBe(0)
+        })
+
+        it('should de-register by a higher-up key', () => {
+          var mockFn = jasmine.createSpy()
+
+          observer.onChange(Getter.fromKeyPath(['foo', 'bar']), mockFn)
+          observer.unwatch(['foo'])
+
+          observer.notifyObservers(initialState.updateIn(['foo', 'bar'], x => {
+            return Map({
+              'baz': 2,
+            })
+          }))
+
+          expect(mockFn.calls.count()).toBe(0)
+        })
+      })
+
+      it('should de-register by Getter', () => {
+        var mockFn = jasmine.createSpy()
+        var getter = Getter.fromKeyPath(['foo'])
+
+        observer.onChange(getter, mockFn)
+        observer.unwatch(getter)
+        observer.notifyObservers(initialState.updateIn(['foo', 'bar'], x => 2))
+
+        expect(mockFn.calls.count()).toBe(0)
+      })
+
+      describe('when two of the same getter are registered', () => {
+        it('should de-register both', () => {
+          var getter = Getter.fromKeyPath(['foo'])
+          var mockFn1 = jasmine.createSpy()
+          var mockFn2 = jasmine.createSpy()
+
+          observer.onChange(getter, mockFn1)
+          observer.onChange(getter, mockFn2)
+          observer.unwatch(getter)
+
+          observer.notifyObservers(initialState.updateIn(['foo', 'bar'], x => 2))
+
+          expect(mockFn1.calls.count()).toBe(0)
+          expect(mockFn2.calls.count()).toBe(0)
+        })
+      })
+
+      describe('when a getter with dependencies is registered', () => {
+        it('should de-register for any changes up the dependency chain', () => {
+          var getter = Getter.fromKeyPath(['foo'])
+          var otherGetter = [getter, Getter.fromKeyPath(['bar'])]
+          var mockFn = jasmine.createSpy()
+
+          observer.onChange(otherGetter, mockFn)
+          observer.unwatch(otherGetter)
+
+          observer.notifyObservers(initialState.updateIn(['foo'], x => 2))
+
+          expect(mockFn.calls.count()).toBe(0)
+        })
+      })
+
+      describe('when a handlerFn is specified', () => {
+        it('should de-register the specific change handler for the getter', () => {
+          var mockFn = jasmine.createSpy()
+          var otherMockFn = jasmine.createSpy()
+          var getter = Getter.fromKeyPath(['foo'])
+
+          observer.onChange(getter, mockFn)
+          observer.onChange(getter, otherMockFn)
+          observer.unwatch(getter, mockFn)
+
+          observer.notifyObservers(initialState.updateIn(['foo'], x => 2))
+
+          expect(mockFn.calls.count()).toEqual(0)
+        })
+
+        it('should not de-register other change handlers for the same getter', () => {
+          var mockFn = jasmine.createSpy()
+          var otherMockFn = jasmine.createSpy()
+          var getter = Getter.fromKeyPath(['foo'])
+
+          observer.onChange(getter, mockFn)
+          observer.onChange(getter, otherMockFn)
+          observer.unwatch(getter, mockFn)
+
+          observer.notifyObservers(initialState.updateIn(['foo'], x => 2))
+
+          expect(otherMockFn.calls.count()).toBe(1)
+        })
+      })
+    })
   })
   // TODO: test the prevValues and registering an observable
 })

tests/getter-tests.js

@@ -31,4 +31,16 @@ describe('Getter', () => {
       }).toThrow()
     })
   })
+
+  describe('wasKeyPath', () => {
+    it('should return true if a KeyPath was converted to a getter', () => {
+      var getter = Getter.fromKeyPath(['foo'])
+      expect(Getter.wasKeyPath(getter)).toBe(true)
+    })
+
+    it('should return false for a non-converted getter', () => {
+      var getter = [['foo'], ['bar'], (foo, bar) => foo + bar]
+      expect(Getter.wasKeyPath(getter)).toBe(false)
+    })
+  })
 })

tests/key-path-tests.js

@@ -1,6 +1,8 @@
 var Immutable = require('immutable')
 var KeyPath = require('../src/key-path')
 var isKeyPath = KeyPath.isKeyPath
+var same = KeyPath.same
+var isUpstream = KeyPath.isUpstream
 
 describe('KeyPath', () => {
   describe('isKeyPath', () => {
@@ -27,4 +29,37 @@ describe('KeyPath', () => {
       expect(isKeyPath([immMap])).toBe(true)
     })
   })
+
+  describe('same', () => {
+    it('should return false when a non-keypath is passed', () => {
+      expect(same(['some', 'keypath'], 'something else')).toBe(false)
+      expect(same({ something: 'else' }, ['some', 'keypath'])).toBe(false)
+    })
+
+    it('should return false when two differnt keypaths are compared', () => {
+      expect(same(['some', 'keypath'], ['some', 'other', 'keypath'])).toBe(false)
+      expect(same(['some', 'keypath'], ['something', 'else'])).toBe(false)
+    })
+
+    it('should return true when the same keypath is compared', () => {
+      expect(same(['some', 'keypath'], ['some', 'keypath'])).toBe(true)
+    })
+  })
+
+  describe('isUpstream', () => {
+    it('should return false when a non-keypath is passed', () => {
+      expect(isUpstream(['some', 'keypath'], 'something else')).toBe(false)
+      expect(isUpstream({ something: 'else' }, ['some', 'keypath'])).toBe(false)
+    })
+
+    it('should return false when two unrelated keypaths are passed', () => {
+      expect(isUpstream(['some', 'keypath'], ['some', 'other', 'keypath'])).toBe(false)
+      expect(isUpstream(['some', 'keypath'], ['something', 'else'])).toBe(false)
+    })
+
+    it('should return true when two related keypaths are passed', () => {
+      expect(isUpstream(['some', 'keypath'], ['some', 'keypath', 'with depth'])).toBe(true)
+      expect(isUpstream(['some'], ['some', 'keypath', 'with depth'])).toBe(true)
+    })
+  })
 })