jimsparkman / RiotControl

Event Controller / Dispatcher For RiotJS, Inspired By Flux
http://jimsparkman.github.io/RiotControl/routing_demo/
MIT License
598 stars 48 forks source link

Event handlers run multiple times when using >1 stores #32

Open vaughnd opened 8 years ago

vaughnd commented 8 years ago

Hi, I'm on riot 2.4.1 and riotcontrol 0.0.3. I'm using webpack to transpile es7.

I'm running into problems with event handlers being fired multiple times after a single event trigger. It seems to be related to my use of multiple stores and riot.observable(this) in the constructor, but I'm not sure how to do it differently. After looking through the other issues, someone suggested using a single broker = riot.observable(); for all stores, but I'm not sure how to get that working.

My handler is definitely added only once, and commenting out the riot.observable(this) in stores reduces the number of times the event is fired.

class AutoCompleteStore {
    constructor() {
        riot.observable(this);
    }
}

let autoCompleteStore = new AutoCompleteStore();
riot.control.addStore(autoCompleteStore);
export default autoCompleteStore;

In my page tag:

let changeResultsHandler = () => {
    console.info("changeResultsHandlerFired");
    this.update();
}
this.on('mount', () => {
    riot.control.on(riot.EVT.changeResults, changeResultsHandler);
    riot.control.trigger(riot.EVT.changeResults);
});

this.on('unmount', () => {
    riot.control.off(riot.EVT.changeResults, changeResultsHandler);
});

Am I using the correct patterns here?

vaughnd commented 8 years ago

After some more debugging it seems that riot.control.on in the tag is calling .on for each store so they all have a copy of the changeResultsHandler... I was assuming that calls to riot.control.on outside of a store, where this.on is used, would bind to a global observable object and not to each store...

vaughnd commented 8 years ago

Gave up and wrote my own riot control. So now I just use riot.control.on/off/once/trigger everywhere, and don't use riot.observable(this) in store classes.

class RiotControl {
  constructor() {
    this._broker = riot.observable();
  }

  on(evt, handler) {
    this._broker.on(evt, handler);
  }

  off(evt, handler) {
    this._broker.off(evt, handler);
  }

  one(evt, handler) {
    this._broker.one(evt, handler);
  }

  trigger() {
    var args = [].slice.call(arguments);
    this._broker.trigger.apply(this._broker, args);
  }
}```
taavisildeberg commented 8 years ago

Can you provide a example how are you using this?

vaughnd commented 8 years ago

Sure, I created a gist here: https://gist.github.com/vaughnd/ddc9130ce86583add3809495acb088ae It's in ES6, but should be easy to adapt.

damusix commented 7 years ago

To be honest, it makes more sense to just use Riot's native observable API instead of wrapping functions around functions around functions. I had been experimenting with RiotControl, which led me to stop using Redux, and after coming across this very issue, I decided it might be better to just drop RiotControl all together. I am now doing this with the same results: https://gist.github.com/damusix/1da99e76895f7d490efec30432adae32

Novalis80 commented 6 years ago

i was running into the same issue. it was related to calling riotControl.trigger('event',data) instead of this.trigger('event',data)

hope it helps somebody