Multiple [Flux.mixins.storeListener] gives "possible EventEmitter memory leak detected. 11 listeners added" about delorean HOT 9 CLOSED

FYI, PR open that solves this issue.

from delorean.

PR merged, closing this issue. @svdoever - please let us know if you see this on our master branch or future releases. thanks!

from delorean.

happens again.

from delorean.

This will happen if you are dispatching a store action and the action handler does not either this.emit('change') or use this.set, which emits a change event automatically. This is because the promise only resolves on a change event. So no change event and the listeners pile up.

from delorean.

Oh okay thank you so much :-) I will have to check a bunch of stores for
this
On Feb 20, 2015 9:13 PM, "Darcy Adams" [email protected] wrote:

This will happen if you are dispatching a store action and the action
handler does not either this.emit('change') or use this.set, which emits
a change event automatically. This is because the promise only resolves on
a change event. So no change event and the listeners pile up.

—
Reply to this email directly or view it on GitHub
#20 (comment).

from delorean.

@darcyadams tracking down a leak like this in our code. We have a bunch of event handlers in stores that do some checks and then return without emitting anything if there is no work to be done...

Am I to understand from this that a change event always needs to be emitted?

from delorean.

@tommoor yes, sadly this is currently the state.

The way delorean has been designed form the start (and it took me a bit to realize this) is that a dispatch('action_name') returns a promise, which is nice. However, in order to resolve that promise, delorean binds to a change event on the store using .once. This works great when the store emits a change event, as the event cleans itself up (by the way, this.set does this automatically). You run into serious problems pretty quickly, however, if you start dispatching actions that do not result in a change event being fired. If said action gets repeatedly dispatched, you have a pretty serious memory leak on your hands (thankfully node's event emitter module is pretty good at console.warning you when this happens).

Ultimately, delorean needs to handle this behind the scenes. The design I have in mind is to resolve the promise after the action has been dispatched to the store. The primary drawback to this approach is the developer will lose the ability to perform any async work in the store before promise gets resolved. IMO, this is not a big deal, as I keep all my async server interactions in the actions layer, but I'm just not sure if it is a common pattern to peform server (or other async work) inside a store action handler.

Now, the other option would be to ditch the promise based architecture all together. This is actually where I am leaning, but wanted to get sign off from @f when he comes back from his military service. Flux is all about one way data flow, and I do not see a large need for the dispatcher to get a promise back from the store on every action. The store gets and action and takes care of updating the state of it's data, the component reacts to the data changing (or not), and then can trigger new data flows the dispatcher. As soon as you get back a promise from the store, the one way data flow breaks.

from delorean.

@darcyadams thanks for the detailed reply! From the code it looks like as long as some other action on the store emits a change then you're good. This would only be an issue if one action without a change event is emitted many times?

from delorean.

@tommoor yes, once the store emits a change, all promises are resolved and the events cleaned up. The issue is a repeatedly dispatching an action or actions that do not result in change events.

from delorean.