AmpersandJS / ampersand-events

Standalone event system that can be mixed into any object
MIT License
18 stars 7 forks source link

passing a context argument to `once` does not apply context to callback #18

Closed dhritzkiv closed 8 years ago

dhritzkiv commented 8 years ago

Here is a proof of the behaviour (or see this notebook on TonicDev)

const AmpersandState = require("ampersand-state");

const Model = AmpersandState.extend({});

const model = new Model();
const someObject = {};

//passing a context argument to `on` correctly applies the context to the callback
model.on("foo", function testThisEqualsModel2() {
    console.log("foo event via `on`; this === someObject");
    console.log(this === someObject);
}, someObject);

//... but doing the same for `once` doesn't work.
model.once("bar", function testThisEqualsModel2() {
    console.log("bar event via `once`; this === someObject")
    console.log(this === someObject);
}, someObject);

model.trigger("foo");
// "foo event via `on`; this === someObject"
// true

model.trigger("bar");
// "bar event via `once`; this === someObject"
// false

The potential problem code in ampersand-events.js (L24):

   once: function (name, callback, context) {
        if (!utils.eventsApi(this, 'once', name, [callback, context]) || !callback) return this;
        var self = this;
        var once = runOnce(function () {
            self.off(name, once);
            callback.apply(self, arguments);
        });
        once._callback = callback;
        return this.on(name, once, context);
    },

Note the callback is being called via apply(self, arguments), instead of something like apply(context, arguments).


This currently affects AmpersandJS/ampersand-view-switcher#35