Skip to content

Commit

Permalink
Optimize view.triggerMethod
Browse files Browse the repository at this point in the history
  • Loading branch information
megawac authored and samccone committed Dec 9, 2014
1 parent 62f15dc commit e5957dd
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 40 deletions.
12 changes: 4 additions & 8 deletions src/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,21 @@ Marionette.Application = Marionette.Object.extend({
this._regionManager = this.getRegionManager();

this.listenTo(this._regionManager, 'before:add:region', function() {
var args = _.toArray(arguments);
this.triggerMethod.apply(this, ['before:add:region'].concat(args));
Marionette._triggerMethod(this, 'before:add:region', arguments);
});

this.listenTo(this._regionManager, 'add:region', function(name, region) {
this[name] = region;
var args = _.toArray(arguments);
this.triggerMethod.apply(this, ['add:region'].concat(args));
Marionette._triggerMethod(this, 'add:region', arguments);
});

this.listenTo(this._regionManager, 'before:remove:region', function() {
var args = _.toArray(arguments);
this.triggerMethod.apply(this, ['before:remove:region'].concat(args));
Marionette._triggerMethod(this, 'before:remove:region', arguments);
});

this.listenTo(this._regionManager, 'remove:region', function(name) {
delete this[name];
var args = _.toArray(arguments);
this.triggerMethod.apply(this, ['remove:region'].concat(args));
Marionette._triggerMethod(this, 'remove:region', arguments);
});
},

Expand Down
5 changes: 2 additions & 3 deletions src/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ Marionette.Controller.extend = Marionette.extend;
// Ensure it can trigger events with Backbone.Events
_.extend(Marionette.Controller.prototype, Backbone.Events, {
destroy: function() {
var args = _.toArray(arguments);
this.triggerMethod.apply(this, ['before:destroy'].concat(args));
this.triggerMethod.apply(this, ['destroy'].concat(args));
Marionette._triggerMethod(this, 'before:destroy', arguments);
Marionette._triggerMethod(this, 'destroy', arguments);

this.stopListening();
this.off();
Expand Down
62 changes: 40 additions & 22 deletions src/trigger-method.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
/* jshint maxstatements: 14, maxcomplexity: 7 */

// Trigger Method
// --------------

// Trigger an event and/or a corresponding method name. Examples:
//
// `this.triggerMethod("foo")` will trigger the "foo" event and
// call the "onFoo" method.
//
// `this.triggerMethod("foo:bar")` will trigger the "foo:bar" event and
// call the "onFooBar" method.
Marionette.triggerMethod = function(event) {

Marionette._triggerMethod = (function() {
// split the event name on the ":"
var splitter = /(^|:)(\w)/gi;

Expand All @@ -19,23 +14,46 @@ Marionette.triggerMethod = function(event) {
return eventName.toUpperCase();
}

// get the method name from the event name
var methodName = 'on' + event.replace(splitter, getEventName);
var method = this[methodName];
var result;
return function(context, event, args) {
var noEventArg = arguments.length < 3;
if (noEventArg) {
args = event;
event = args[0];
}

// call the onMethodName if it exists
if (_.isFunction(method)) {
// pass all arguments, except the event name
result = method.apply(this, _.tail(arguments));
}
// get the method name from the event name
var methodName = 'on' + event.replace(splitter, getEventName);
var method = context[methodName];
var result;

// trigger the event, if a trigger method exists
if (_.isFunction(this.trigger)) {
this.trigger.apply(this, arguments);
}
// call the onMethodName if it exists
if (_.isFunction(method)) {
// pass all args, except the event name
result = method.apply(context, noEventArg ? _.rest(args) : args);
}

return result;
// trigger the event, if a trigger method exists
if (_.isFunction(context.trigger)) {
if (noEventArg + args.length > 1) {
context.trigger.apply(context, noEventArg ? args : [event].concat(_.rest(args, 0)));
} else {
context.trigger(event);
}
}

return result;
};
})();

// Trigger an event and/or a corresponding method name. Examples:
//
// `this.triggerMethod("foo")` will trigger the "foo" event and
// call the "onFoo" method.
//
// `this.triggerMethod("foo:bar")` will trigger the "foo:bar" event and
// call the "onFooBar" method.
Marionette.triggerMethod = function(event) {
return Marionette._triggerMethod(this, arguments);
};

// triggerMethodOn invokes triggerMethod on a specific context
Expand Down
14 changes: 7 additions & 7 deletions src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,13 @@ Marionette.View = Backbone.View.extend({
// import the `triggerMethod` to trigger events with corresponding
// methods if the method exists
triggerMethod: function() {
var args = arguments;
var triggerMethod = Marionette.triggerMethod;

var ret = triggerMethod.apply(this, args);
_.each(this._behaviors, function(b) {
triggerMethod.apply(b, args);
});
var triggerMethod = Marionette._triggerMethod;
var ret = triggerMethod(this, arguments);
var behaviors = this._behaviors;
// Use good ol' for as this is a very hot function
for (var i = 0, length = behaviors && behaviors.length; i < length; i++) {
triggerMethod(behaviors[i], arguments);
}

return ret;
},
Expand Down
4 changes: 4 additions & 0 deletions test/unit/collection-view.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,10 @@ describe('collection view', function() {
expect(_.size(this.collectionView.children)).to.equal(0);
});

it('should not affect the underlying collection', function() {
expect(this.collection).to.have.length(1);
});

it('should unbind any listener to custom view events', function() {
expect(this.collectionView.stopListening).to.have.been.called;
});
Expand Down
13 changes: 13 additions & 0 deletions test/unit/region.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,8 @@ describe('region', function() {
this.setFixtures('<div id="region"></div>');
this.beforeEmptySpy = new sinon.spy();
this.emptySpy = new sinon.spy();
this.onBeforeDestroy = this.sinon.stub();
this.onDestroy = this.sinon.stub();

this.region = new Backbone.Marionette.Region({
el: '#region'
Expand All @@ -876,6 +878,9 @@ describe('region', function() {

this.view = new this.View();

this.view.on('before:destroy', this.onBeforeDestroy);
this.view.on('destroy', this.onDestroy);

this.region.show(this.view);
this.region.currentView.destroy();
});
Expand All @@ -885,6 +890,14 @@ describe('region', function() {
expect(this.emptySpy).to.have.been.calledOnce.calledWith(this.view);
expect(this.region.currentView).to.be.undefined;
});

it('view "before:destroy" event is triggered once', function() {
expect(this.onBeforeDestroy).to.have.been.calledOnce;
});

it('view "destroy" event is triggered once', function() {
expect(this.onDestroy).to.have.been.calledOnce;
});
});

describe('when showing undefined in a region', function() {
Expand Down

0 comments on commit e5957dd

Please sign in to comment.