jamesmacaulay / react-bacon

A little module for using React with Bacon.js
MIT License
118 stars 7 forks source link

this.eventStream is not cached correctly #3

Closed w33ble closed 10 years ago

w33ble commented 10 years ago

My understanding is that this.eventStream should instantiate a new Bus if the stream wasn't created, and returned the cached one when it's called again. I was going to submit a PR, but wanted to confirm that I was right - if I'm wrong I guess you can just close this issue.

In the following code:

eventStream: function(eventName) {
      var bacon = this._bacon = this._bacon || {};
      var buses = bacon['buses.events'] = bacon['buses.events'] || {};
      var bus = buses[eventName];
      if (!bus) {
        bus = buses[eventName] = new Bacon.Bus();
        this[eventName] = function sendEventToStream(event) {
          bus.push(event);
        };
      }
      return bus;
}

The if(!bus) is always true, since bus doesn't seem to be cached correctlyin the buses.events collection.

The result is that you must cache what is returned from this.eventStream('someEvent') in order to attach handlers to it. That's easy enough, but an annoying side effect is that calling this.eventStream('someEvent') again will clobber any previously created event busses and handlers you added.

jamesmacaulay commented 10 years ago

I believe this bug was fixed with #1, but I neglected to publish a new version to npm. Were you testing with the old version by any chance?

I just published a new version (0.0.3) to npm, updated the UMD package in /dist, and updated the jsFiddle linked in the README. Is this still an issue?

w33ble commented 10 years ago

Yup, looks like I had an old version. Just pulled down the latest and everything is working as expected. Didn't even think to try grabbing the current version here. Thanks!