Skip to content

Commit

Permalink
Merge pull request #222 from flightjs/clean_up_bound_array
Browse files Browse the repository at this point in the history
'off' callback may be a bound or an unbound function
  • Loading branch information
sayrer committed Feb 18, 2014
2 parents 1ff84d1 + 73fcf91 commit 7514a91
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ You will have to reference Flight's installed dependencies –
```html
<script src="bower_components/es5-shim/es5-shim.js"></script>
<script src="bower_components/es5-shim/es5-sham.js"></script>
<script src="bower_components/jquery/jquery.js"></script>
<script src="bower_components/jquery/dist/jquery.js"></script>
<script data-main="main.js" src="bower_components/requirejs/require.js"></script>
...
```
Expand Down
2 changes: 1 addition & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = function (config) {
// loaded without require
'bower_components/es5-shim/es5-shim.js',
'bower_components/es5-shim/es5-sham.js',
'bower_components/jquery/jquery.js',
'bower_components/jquery/dist/jquery.js',
'build/flight.js',

// hack to load RequireJS after the shim libs
Expand Down
4 changes: 3 additions & 1 deletion lib/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ define(
}

if (callback) {
//this callback may be the original function or a bound version
var boundFunctions = callback.target ? callback.target.bound : callback.bound || [];
//set callback to version bound against this instance
callback.bound && callback.bound.some(function(fn, i, arr) {
boundFunctions && boundFunctions.some(function(fn, i, arr) {
if (fn.context && (this.identity == fn.context.identity)) {
arr.splice(i, 1);
callback = fn;
Expand Down
52 changes: 47 additions & 5 deletions test/spec/events_spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

define(['lib/component'], function (defineComponent) {
define(['lib/component', 'lib/registry'], function (defineComponent, registry) {

describe("(Core) events", function () {
var Component = (function () {
Expand All @@ -9,7 +9,6 @@ define(['lib/component'], function (defineComponent) {
this.after('initialize', function () {
this.testString || (this.testString = "");
this.testString += "-initBase-";
this.on(document, 'exampleDocumentEvent', this.exampleMethod);
});
}

Expand Down Expand Up @@ -168,6 +167,30 @@ define(['lib/component'], function (defineComponent) {
expect(spy).toHaveBeenCalled();
});

it('removes the binding when "off" is supplied with a bound callback', function () {
var instance1 = (new Component).initialize(window.outerDiv);
var spy = jasmine.createSpy();
instance1.on(document, 'event1', spy);
var boundCb = registry.findInstanceInfo(instance1).events.filter(function(e) {
return e.type=="event1"
})[0].callback;
instance1.off(document, 'event1', boundCb);
instance1.trigger('event1');
expect(spy).not.toHaveBeenCalled();
});

it('retains one binding when another is removed for multiple registered events for the same callback function when "off" is supplied with a bound callback', function () {
var instance1 = (new Component).initialize(window.outerDiv);
var spy = jasmine.createSpy();
instance1.on(document, 'event1', spy);
instance1.on(document, 'event2', spy);
var boundCb = registry.findInstanceInfo(instance1).events.filter(function(e) {
return e.type=="event1"
})[0].callback;
instance1.off(document, 'event1', boundCb);
instance1.trigger('event2');
expect(spy).toHaveBeenCalled();
});

it('does not unbind those registered events that share a callback, but were not sent "off" requests', function () {
var instance1 = (new Component).initialize(window.outerDiv);
Expand All @@ -179,12 +202,31 @@ define(['lib/component'], function (defineComponent) {
expect(spy).toHaveBeenCalled();
});

it('does not unbind those registered events that share a callback, but were not sent "off" requests (when "off" is supplied with a bound callback)', function () {
var instance1 = (new Component).initialize(window.outerDiv);
var instance2 = (new Component).initialize(window.anotherDiv);
var spy1 = jasmine.createSpy();
instance1.on(document, 'event1', spy1);
var spy2 = jasmine.createSpy();
instance1.on(document, 'event1', spy2);
var boundCb = registry.findInstanceInfo(instance1).events.filter(function(e) {
return e.type=="event1"
})[0].callback;
instance1.off(document, 'event1', boundCb);
instance1.trigger('event1');
expect(spy2).toHaveBeenCalled();
});

it('does not unbind event handlers which share a node but were registered by different instances', function () {
var instance1 = (new Component).initialize(window.outerDiv);
var instance2 = (new Component).initialize(window.anotherDiv);
instance1.off(document, 'exampleDocumentEvent', instance1.exampleMethod);
instance1.trigger('exampleDocumentEvent');
expect(instance2.exampleMethod).toHaveBeenCalled();
var spy1 = jasmine.createSpy();
instance1.on(document, 'event1', spy1);
var spy2 = jasmine.createSpy();
instance1.on(document, 'event1', spy2);
instance1.off(document, 'event1', spy1);
instance1.trigger('event1');
expect(spy2).toHaveBeenCalled();
});

it('bubbles custom events between components', function () {
Expand Down

0 comments on commit 7514a91

Please sign in to comment.