feat(observable): BREAKING return disposer instead of context for chaining

src/mixins/observable.mixin.js

@@ -24,8 +24,7 @@
    * @alias on
    * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler})
    * @param {Function} handler Function that receives a notification when an event of the specified type occurs
-   * @return {Self} thisArg
-   * @chainable
+   * @return {Function} disposer
    */
   function on(eventName, handler) {
     if (!this.__eventListeners) {
@@ -43,7 +42,7 @@
       }
       this.__eventListeners[eventName].push(handler);
     }
-    return this;
+    return off.bind(this, eventName, handler);
   }
 
   function _once(eventName, handler) {
@@ -52,19 +51,30 @@
       this.off(eventName, _handler);
     }.bind(this);
     this.on(eventName, _handler);
+    return _handler;
   }
 
+  /**
+   * Observes specified event **once**
+   * @memberOf fabric.Observable
+   * @alias once
+   * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler})
+   * @param {Function} handler Function that receives a notification when an event of the specified type occurs
+   * @return {Function} disposer
+   */
   function once(eventName, handler) {
     // one object with key/value pairs was passed
     if (arguments.length === 1) {
+      var handlers = {};
       for (var prop in eventName) {
-        _once.call(this, prop, eventName[prop]);
+        handlers[prop] = _once.call(this, prop, eventName[prop]);
       }
+      return off.bind(this, handlers);
     }
     else {
-      _once.call(this, eventName, handler);
+      var _handler = _once.call(this, eventName, handler);
+      return off.bind(this, eventName, _handler);
     }
-    return this;
   }
 
   /**
@@ -74,12 +84,10 @@
    * @alias off
    * @param {String|Object} eventName Event name (eg. 'after:render') or object with key/value pairs (eg. {'after:render': handler, 'selection:cleared': handler})
    * @param {Function} handler Function to be deleted from EventListeners
-   * @return {Self} thisArg
-   * @chainable
    */
   function off(eventName, handler) {
     if (!this.__eventListeners) {
-      return this;
+      return;
     }
 
     // remove all key/value pairs (event name -> event handler)
@@ -89,33 +97,30 @@
       }
     }
     // one object with key/value pairs was passed
-    else if (arguments.length === 1 && typeof arguments[0] === 'object') {
+    else if (typeof eventName === 'object' && typeof handler === 'undefined') {
       for (var prop in eventName) {
         _removeEventListener.call(this, prop, eventName[prop]);
       }
     }
     else {
       _removeEventListener.call(this, eventName, handler);
     }
-    return this;
   }
 
   /**
    * Fires event with an optional options object
    * @memberOf fabric.Observable
    * @param {String} eventName Event name to fire
    * @param {Object} [options] Options object
-   * @return {Self} thisArg
-   * @chainable
    */
   function fire(eventName, options) {
     if (!this.__eventListeners) {
-      return this;
+      return;
     }
 
     var listenersForEvent = this.__eventListeners[eventName];
     if (!listenersForEvent) {
-      return this;
+      return;
     }
 
     for (var i = 0, len = listenersForEvent.length; i < len; i++) {
@@ -124,7 +129,6 @@
     this.__eventListeners[eventName] = listenersForEvent.filter(function(value) {
       return value !== false;
     });
-    return this;
   }
 
   /**

test/unit/object.js

@@ -579,29 +579,16 @@
     }, 1000);
   });
 
-  QUnit.test('on off fire are chainable', function(assert) {
-    var object = new fabric.Object({ left: 20, top: 30, width: 40, height: 50, angle: 43 });
-    var ret;
-    ret = object.fire('');
-    assert.equal(ret, object, 'fire is chainable when no events are registered at all');
-    ret = object.on('hi', function() {});
-    assert.equal(ret, object, 'on is chainable');
-    ret = object.fire('bye');
-    assert.equal(ret, object, 'fire is chainable when firing a non registerd event');
-    ret = object.fire('hi');
-    assert.equal(ret, object, 'fire is chainable when firing a registerd event');
-    ret = object.off('hi');
-    assert.equal(ret, object, 'off is chainable');
-  });
-
   QUnit.test('observable', function(assert) {
     var object = new fabric.Object({ left: 20, top: 30, width: 40, height: 50, angle: 43 });
 
     var fooFired = false,
         barFired = false;
 
-    object.on('foo', function() { fooFired = true; });
-    object.on('bar', function() { barFired = true; });
+    var fooDisposer = object.on('foo', function() { fooFired = true; });
+    var barDisposer = object.on('bar', function () { barFired = true; });
+    assert.ok(typeof fooDisposer === 'function', 'should return disposer');
+    assert.ok(typeof barDisposer === 'function', 'should return disposer');
 
     object.fire('foo');
     assert.ok(fooFired);

test/unit/observable.js

@@ -326,30 +326,59 @@ QUnit.test('adding events', function(assert) {
 });
 
 
-QUnit.test('chaining', function(assert) {
+QUnit.test('disposing', function(assert) {
   var foo = { };
   fabric.util.object.extend(foo, fabric.Observable);
 
-  var event1Fired = false, event2Fired = false;
-  foo
-    .on('event1', function() {
-      event1Fired = true;
-    })
-    .on('event2', function() {
-      event2Fired = true;
+  var fired = new Array(7).fill(false);
+  var getEventName = function (index) {
+    return `event${index + 1}`;
+  }
+  var createHandler = function (index) {
+    return function () {
+      fired[index] = true;
+    }
+  }
+  var attach = function () {
+    return [
+      foo.on(getEventName(0), createHandler(0)),
+      foo.on(getEventName(1), createHandler(1)),
+      foo.once(getEventName(2), createHandler(2)),
+      foo.on({
+        [getEventName(3)]: createHandler(3),
+        [getEventName(4)]: createHandler(4),
+      }),
+      foo.once({
+        [getEventName(5)]: createHandler(5),
+        [getEventName(6)]: createHandler(6),
+      })
+    ];
+  }
+  var disposers = [];
+  var fireAll = function () {
+    fired.forEach(function (__, index) {
+      foo.fire(getEventName(index));
     });
+  }
+  var dispose = function () {
+    disposers.forEach(function (disposer) {
+      disposer();
+    });
+  }
+
+  //  dispose before firing
+  disposers = attach();
+  dispose();
+  fireAll();
+  assert.deepEqual(fired, new Array(fired.length).fill(false));
+
+  //  dispose after firing
+  disposers = attach();
+  fireAll();
+  assert.deepEqual(fired, new Array(fired.length).fill(true));
+  fired = new Array(fired.length).fill(false);
+  dispose();
+  fireAll();
+  assert.deepEqual(fired, new Array(fired.length).fill(false));
 
-  foo.fire('event2').fire('event1');
-
-  assert.equal(event1Fired, true);
-  assert.equal(event2Fired, true);
-
-  event1Fired = false;
-  event2Fired = false;
-
-  foo.off('event1').off('event2');
-  foo.fire('event2').fire('event1');
-
-  assert.equal(event1Fired, false);
-  assert.equal(event2Fired, false);
 });