Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Events.trigger with arrays #12

Open
megawac opened this issue Aug 9, 2013 · 5 comments
Open

Events.trigger with arrays #12

megawac opened this issue Aug 9, 2013 · 5 comments

Comments

@megawac
Copy link
Contributor

megawac commented Aug 9, 2013

When a event is triggered, eg model property change, if the args is an array, events is going to apply each listener with the array. With all other data types the listeners will be applied with an array containing the value.

For example:
myModel.set("x", {a:1,b:2})
calls all listeners with first argument: args({a:1,b:2}).

However
myModel.set("y", [1,2])
calls all listeners with arguments: args(1, 2) instead of args([1,2])

Hope that makes sense I'm working on a fix.. For now I just changed trigger a bit with this dirty line :( :
args = Type.isArray(args) ? [args] : Array.from(args);

@DimitarChristoff
Copy link
Member

hrm this is due to the .apply() on the event emitter which expands the array. will look at changing this to a .call and what options that presents. i guess you could do model.set.call(model, y, [1,2]) as well, but that's rubbish. i am currently on a contract in China so my development availability is poor, will try to work on this tomorrow night. any chance you could make a branch and a pull request with a change to the tests.

this is for collection evnet bubbling? this test passes, which is:

https://github.com/epitome-mvc/Epitome/blob/master/test/tests/epitome-events-test.js#L109-L120

perhaps it needs to be changed, though it may be somewhat breaking. the events fire exactly like in MooTools Class.Events but I can override them since I have a local copy reimplementing them for versatility.

@megawac
Copy link
Contributor Author

megawac commented Aug 11, 2013

Never noticed Mootools Events has the same behaviour with Arrays - I've always called them with objects. Anyway I'll cleanup my changes in my fork and you can decide how you want the behaviour to function.

@megawac
Copy link
Contributor Author

megawac commented Aug 16, 2013

btw is array.flatten what you want to do for passing events? Itll run into the same issue. Unless I'm overlooking something I'd write the function like this:

trigger: function(type, args){
                type = removeOn(type);
    var events = this.$events[type] || [],
        subs = (type in this.$subscribers) ? this.$subscribers[type] : (all in this.$subscribers) ? this.$subscribers[all] : [],
        self = this;

    if (!events && !subs) return this;
    args = Array.from(args);

    events.each(function(fn){
        // local events
        fn.apply(self, args);
    });

    subs.each(function(sub){
        // if event was added towards a specific callback, fire that
        if (sub.fn){
            sub.fn.apply(sub.context, args);
        }
        else {
            // runs on subscriber, shifting arguments to pass on instance with a fake event object.

            // this use is not recommended as it can cause event storms, use with caution and
            // argument shift, arg1 = context. result of .listenTo(obj) with no other args or with type but no callback.
            sub.subscriber.trigger(type, [self].append(Array.from(args));//move out of loop?
        }
    });

    return this;
}

@DimitarChristoff
Copy link
Member

this is now slightly different in the new event imlementation. https://github.com/DimitarChristoff/epic/blob/master/lib/index.js#L33-L148

@megawac
Copy link
Contributor Author

megawac commented Dec 28, 2013

That's essentially how I ended up deciding to implement it as well megawac@2f1f5fd#diff-f05ace420865a960e67f1030e352afd8R48

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants