From e41e8a886a284d3601a97e16599209529b50c29c Mon Sep 17 00:00:00 2001 From: iirvine Date: Fri, 12 Apr 2013 15:21:48 -0700 Subject: [PATCH 1/6] initial implementation of events#once --- spec/suites/core/EventsSpec.js | 62 ++++++++++++++++++++++++++++++++++ src/core/Events.js | 23 +++++++++++++ 2 files changed, 85 insertions(+) diff --git a/spec/suites/core/EventsSpec.js b/spec/suites/core/EventsSpec.js index fc0976d0..48af81d1 100644 --- a/spec/suites/core/EventsSpec.js +++ b/spec/suites/core/EventsSpec.js @@ -245,4 +245,66 @@ describe('Events', function() { expect(spy3.called).to.be(true); }); }); + + describe('#once', function(){ + it('removes event listeners after first fire', function() { + var obj = new Klass(), + spy = new sinon.spy(); + + obj.once('test', spy); + obj.fire('test'); + + expect(spy.called).to.be(true); + + obj.fire('test'); + + expect(spy.callCount).to.be.lessThan(2); + }); + + it('works with object hash', function() { + var obj = new Klass(), + spy = new sinon.spy(), + otherSpy = new sinon.spy(); + + obj.once({ + test: spy, + otherTest: otherSpy + }); + + obj.fire('test'); + obj.fire('otherTest'); + + expect(spy.called).to.be(true); + expect(otherSpy.called).to.be(true); + + obj.fire('test'); + obj.fire('otherTest'); + + expect(spy.callCount).to.be.lessThan(2); + expect(otherSpy.callCount).to.be.lessThan(2); + }); + + it('only removes the fired event handler', function(){ + var obj = new Klass(), + spy = new sinon.spy(), + otherSpy = new sinon.spy(); + + obj.once({ + test: spy, + otherTest: otherSpy + }); + + obj.fire('test'); + + expect(spy.called).to.be(true); + expect(otherSpy.called).to.be(false); + + obj.fire('otherTest'); + + expect(otherSpy.called).to.be(true); + + expect(spy.callCount).to.be.lessThan(2); + expect(otherSpy.callCount).to.be.lessThan(2); + }); + }); }); diff --git a/src/core/Events.js b/src/core/Events.js index 27910eae..6f3de7dd 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -148,6 +148,29 @@ L.Mixin.Events = { } return this; + }, + + once: function(types, fn, context) { + handlerFor = function(fn, type, context) { + var handler = function() { + this.removeEventListener(type, fn, context); + this.removeEventListener(type, handler, context); + } + return handler; + } + + if (typeof types === 'object') { + for (type in types) { + if (types.hasOwnProperty(type)) { + this.addEventListener(type, types[type], fn); + this.addEventListener(type, handlerFor(types[type], type, fn), fn); + } + } + return this; + } + + this.addEventListener(types, fn, context); + return this.addEventListener(types, handlerFor(fn, types, context), context); } }; From e25f730b049420dce1c3afb072043ab9c92ffb5c Mon Sep 17 00:00:00 2001 From: iirvine Date: Fri, 12 Apr 2013 17:03:14 -0700 Subject: [PATCH 2/6] better implementation, more tests --- spec/suites/core/EventsSpec.js | 69 ++++++++++++++++++++++++++++++++-- src/core/Events.js | 31 ++++++++------- 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/spec/suites/core/EventsSpec.js b/spec/suites/core/EventsSpec.js index 48af81d1..b9706fb3 100644 --- a/spec/suites/core/EventsSpec.js +++ b/spec/suites/core/EventsSpec.js @@ -246,12 +246,12 @@ describe('Events', function() { }); }); - describe('#once', function(){ + describe('#once', function() { it('removes event listeners after first fire', function() { var obj = new Klass(), spy = new sinon.spy(); - obj.once('test', spy); + obj.once('test', spy, obj); obj.fire('test'); expect(spy.called).to.be(true); @@ -269,7 +269,7 @@ describe('Events', function() { obj.once({ test: spy, otherTest: otherSpy - }); + }, obj); obj.fire('test'); obj.fire('otherTest'); @@ -284,7 +284,7 @@ describe('Events', function() { expect(otherSpy.callCount).to.be.lessThan(2); }); - it('only removes the fired event handler', function(){ + it('only removes the fired event handler', function() { var obj = new Klass(), spy = new sinon.spy(), otherSpy = new sinon.spy(); @@ -306,5 +306,66 @@ describe('Events', function() { expect(spy.callCount).to.be.lessThan(2); expect(otherSpy.callCount).to.be.lessThan(2); }); + + it('can be passed multiple types', function() { + var obj = new Klass(), + spy = new sinon.spy(), + otherSpy = new sinon.spy(); + + obj.once('test otherTest', spy, obj); + obj.fire('otherTest'); + + expect(spy.called).to.be(true); + + obj.fire('test'); + + expect(spy.callCount).to.be.lessThan(2); + }); + + it('provides event object to listeners and executes with the correct context', function() { + var obj = new Klass(), + obj2 = new Klass(), + obj3 = new Klass(), + obj4 = new Klass(), + foo = new Klass(); + + function listener1(e) { + expect(e.type).to.eql('test'); + expect(e.target).to.eql(obj); + expect(this).to.eql(obj); + expect(e.baz).to.eql(1); + } + + function listener2(e) { + expect(e.type).to.eql('test'); + expect(e.target).to.eql(obj2); + expect(this).to.eql(foo); + expect(e.baz).to.eql(2); + } + + function listener3(e) { + expect(e.type).to.eql('test'); + expect(e.target).to.eql(obj3); + expect(this).to.eql(obj3); + expect(e.baz).to.eql(3); + } + + function listener4(e) { + expect(e.type).to.eql('test'); + expect(e.target).to.eql(obj4); + expect(this).to.eql(foo); + expect(e.baz).to.eql(4); + } + + obj.once('test', listener1); + obj2.once('test', listener2, foo); + obj3.once({ test: listener3 }); + obj4.once({ test: listener4 }, foo); + + obj.fireEvent('test', {baz: 1}); + obj2.fireEvent('test', {baz: 2}); + obj3.fireEvent('test', {baz: 3}); + obj4.fireEvent('test', {baz: 4}); + }); }); }); diff --git a/src/core/Events.js b/src/core/Events.js index 6f3de7dd..93cde26b 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -115,10 +115,7 @@ L.Mixin.Events = { return this; } - var event = L.Util.extend({}, data, { - type: type, - target: this - }); + var event = L.Util.extend({}, data, { type: type, target: this }); var listeners, i, len, eventsObj, contextId; @@ -151,26 +148,34 @@ L.Mixin.Events = { }, once: function(types, fn, context) { - handlerFor = function(fn, type, context) { - var handler = function() { - this.removeEventListener(type, fn, context); - this.removeEventListener(type, handler, context); + var once = function(fn) { + var done = false; + return function() { + if (done) return; + fn.apply(this, arguments); + fn = null; + done = true; } - return handler; } + var handlerFor = L.bind(function(type, fn, context) { + var handler = once(function() { + this.removeEventListener(types, handler); + fn.apply(context || this, arguments); + }); + return handler; + }, this); + if (typeof types === 'object') { for (type in types) { if (types.hasOwnProperty(type)) { - this.addEventListener(type, types[type], fn); - this.addEventListener(type, handlerFor(types[type], type, fn), fn); + this.addEventListener(type, handlerFor(type, types[type], fn), fn); } } return this; } - this.addEventListener(types, fn, context); - return this.addEventListener(types, handlerFor(fn, types, context), context); + return this.addEventListener(types, handlerFor(types,fn,context), context); } }; From a469a387a16398253dcd74d0872edbdb1d0e215f Mon Sep 17 00:00:00 2001 From: iirvine Date: Sat, 13 Apr 2013 22:11:01 -0700 Subject: [PATCH 3/6] back to first implementation to fix event leak --- spec/suites/core/EventsSpec.js | 90 ++++------------------------------ src/core/Events.js | 44 +++++++---------- 2 files changed, 27 insertions(+), 107 deletions(-) diff --git a/spec/suites/core/EventsSpec.js b/spec/suites/core/EventsSpec.js index b9706fb3..f8554eb1 100644 --- a/spec/suites/core/EventsSpec.js +++ b/spec/suites/core/EventsSpec.js @@ -247,7 +247,7 @@ describe('Events', function() { }); describe('#once', function() { - it('removes event listeners after first fire', function() { + it('removes event listeners after first trigger', function() { var obj = new Klass(), spy = new sinon.spy(); @@ -261,7 +261,7 @@ describe('Events', function() { expect(spy.callCount).to.be.lessThan(2); }); - it('works with object hash', function() { + it('works with an object hash', function() { var obj = new Klass(), spy = new sinon.spy(), otherSpy = new sinon.spy(); @@ -284,88 +284,16 @@ describe('Events', function() { expect(otherSpy.callCount).to.be.lessThan(2); }); - it('only removes the fired event handler', function() { - var obj = new Klass(), - spy = new sinon.spy(), - otherSpy = new sinon.spy(); + it("doesn't call listeners to events that have been removed", function () { + var obj = new Klass(), + spy = new sinon.spy(); - obj.once({ - test: spy, - otherTest: otherSpy - }); + obj.once('test', spy, obj); + obj.off('test', spy, obj); - obj.fire('test'); + obj.fire('test'); - expect(spy.called).to.be(true); - expect(otherSpy.called).to.be(false); - - obj.fire('otherTest'); - - expect(otherSpy.called).to.be(true); - - expect(spy.callCount).to.be.lessThan(2); - expect(otherSpy.callCount).to.be.lessThan(2); - }); - - it('can be passed multiple types', function() { - var obj = new Klass(), - spy = new sinon.spy(), - otherSpy = new sinon.spy(); - - obj.once('test otherTest', spy, obj); - obj.fire('otherTest'); - - expect(spy.called).to.be(true); - - obj.fire('test'); - - expect(spy.callCount).to.be.lessThan(2); - }); - - it('provides event object to listeners and executes with the correct context', function() { - var obj = new Klass(), - obj2 = new Klass(), - obj3 = new Klass(), - obj4 = new Klass(), - foo = new Klass(); - - function listener1(e) { - expect(e.type).to.eql('test'); - expect(e.target).to.eql(obj); - expect(this).to.eql(obj); - expect(e.baz).to.eql(1); - } - - function listener2(e) { - expect(e.type).to.eql('test'); - expect(e.target).to.eql(obj2); - expect(this).to.eql(foo); - expect(e.baz).to.eql(2); - } - - function listener3(e) { - expect(e.type).to.eql('test'); - expect(e.target).to.eql(obj3); - expect(this).to.eql(obj3); - expect(e.baz).to.eql(3); - } - - function listener4(e) { - expect(e.type).to.eql('test'); - expect(e.target).to.eql(obj4); - expect(this).to.eql(foo); - expect(e.baz).to.eql(4); - } - - obj.once('test', listener1); - obj2.once('test', listener2, foo); - obj3.once({ test: listener3 }); - obj4.once({ test: listener4 }, foo); - - obj.fireEvent('test', {baz: 1}); - obj2.fireEvent('test', {baz: 2}); - obj3.fireEvent('test', {baz: 3}); - obj4.fireEvent('test', {baz: 4}); + expect(spy.called).to.be(false); }); }); }); diff --git a/src/core/Events.js b/src/core/Events.js index 93cde26b..4d017b49 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -148,34 +148,26 @@ L.Mixin.Events = { }, once: function(types, fn, context) { - var once = function(fn) { - var done = false; - return function() { - if (done) return; - fn.apply(this, arguments); - fn = null; - done = true; - } - } + handlerFor = function (fn, type, context) { + var handler = function () { + this.removeEventListener(type, fn, context); + this.removeEventListener(type, handler, context); + } + return handler; + } - var handlerFor = L.bind(function(type, fn, context) { - var handler = once(function() { - this.removeEventListener(types, handler); - fn.apply(context || this, arguments); - }); - return handler; - }, this); + if (typeof types === 'object') { + for (type in types) { + if (types.hasOwnProperty(type)) { + this.addEventListener(type, types[type], fn); + this.addEventListener(type, handlerFor(types[type], type, fn), fn); + } + } + return this; + } - if (typeof types === 'object') { - for (type in types) { - if (types.hasOwnProperty(type)) { - this.addEventListener(type, handlerFor(type, types[type], fn), fn); - } - } - return this; - } - - return this.addEventListener(types, handlerFor(types,fn,context), context); + this.addEventListener(types, fn, context); + return this.addEventListener(types, handlerFor(fn, types, context), context); } }; From ffe1d79671a26ec6a826c3d9d2c9c9d56c811274 Mon Sep 17 00:00:00 2001 From: iirvine Date: Sun, 14 Apr 2013 16:30:03 -0700 Subject: [PATCH 4/6] damn my coffeescript brainfarts --- src/core/Events.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Events.js b/src/core/Events.js index 4d017b49..bfe721fd 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -148,7 +148,7 @@ L.Mixin.Events = { }, once: function(types, fn, context) { - handlerFor = function (fn, type, context) { + var handlerFor = function (fn, type, context) { var handler = function () { this.removeEventListener(type, fn, context); this.removeEventListener(type, handler, context); From 76140994e40119cc0ec4f3144fb606d96834c735 Mon Sep 17 00:00:00 2001 From: iirvine Date: Sun, 14 Apr 2013 16:36:05 -0700 Subject: [PATCH 5/6] fixing jshint errors --- src/core/Events.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Events.js b/src/core/Events.js index bfe721fd..26f08770 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -147,17 +147,17 @@ L.Mixin.Events = { return this; }, - once: function(types, fn, context) { + once: function (types, fn, context) { var handlerFor = function (fn, type, context) { var handler = function () { this.removeEventListener(type, fn, context); this.removeEventListener(type, handler, context); - } + }; return handler; - } + }; if (typeof types === 'object') { - for (type in types) { + for (var type in types) { if (types.hasOwnProperty(type)) { this.addEventListener(type, types[type], fn); this.addEventListener(type, handlerFor(types[type], type, fn), fn); From 031086681f56427003df3d95a9db28d28df632c3 Mon Sep 17 00:00:00 2001 From: iirvine Date: Sun, 14 Apr 2013 18:47:53 -0700 Subject: [PATCH 6/6] fix our context to the object that's registering this listener --- spec/suites/core/EventsSpec.js | 12 ++++++++++++ src/core/Events.js | 8 ++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/spec/suites/core/EventsSpec.js b/spec/suites/core/EventsSpec.js index f8554eb1..b11315ab 100644 --- a/spec/suites/core/EventsSpec.js +++ b/spec/suites/core/EventsSpec.js @@ -295,5 +295,17 @@ describe('Events', function() { expect(spy.called).to.be(false); }); + + it('works if called from a context that doesnt implement #Events', function() { + var obj = new Klass(), + spy = new sinon.spy(), + foo = {}; + + obj.once('test', spy, foo); + + obj.fire('test'); + + expect(spy.called).to.be(true); + }); }); }); diff --git a/src/core/Events.js b/src/core/Events.js index 26f08770..f8a73e91 100644 --- a/src/core/Events.js +++ b/src/core/Events.js @@ -148,13 +148,13 @@ L.Mixin.Events = { }, once: function (types, fn, context) { - var handlerFor = function (fn, type, context) { - var handler = function () { + var handlerFor = L.bind(function (fn, type, context) { + var handler = L.bind(function () { this.removeEventListener(type, fn, context); this.removeEventListener(type, handler, context); - }; + }, this); return handler; - }; + }, this); if (typeof types === 'object') { for (var type in types) {