model.on() and model.removeAllListeners() issue - multiplication of listeners

[ile]

At http://derbyjs.com/#events :

Models inherit from the standard Node.js EventEmitter, and they support the same methods: on, once, removeListener, emit, etc.

Issue 1:

I'm trying to add some listeners with model.on(), which works well until I do the same thing again (ie. click a link that renders the same route which adds the same listeners) : these listeners will get added again in the path, doubling the said listeners in the path. And every time I render the same route they will get added, ending up having multiple copies of the same listeners in the same path. This seems an obvious problem to me.

Issue 2:

To work around that, I'm using removeAllListeners() first. But, that seems to remove some Derby's/Racer's internal listeners as well, and real-time updates on the page don't happen after that.

So - the main problem is the multiplication of the listeners. If there is a way to do this correctly, please let me know? Otherwise maybe this is an issue to be fixed? TY.

[SLaks]

This behavior is by design.
When you click a link within the app, the route handler runs on the client, without resetting anything from previous runs.

If you want to maintain model listeners, you can either remove them (one-by-one) when leaving the page (I ended up writing my own pages framework to maintain & dispose state used by different areas of the app), or add them only once in app.ready() and keep them forever.

Note that if you simply add listeners in a route handler, they won't apply when you first load the page, since the route handler will only run on the server.

[ile]

Thank you for the info @SLaks, now that I think of it, a single listener in the app.ready() might work in this case.

I was adding the listeners into the scoped models, but that seems to lead to problems. Maybe this is generally true - listeners and scoped models aren't going to work very well together? If so, maybe a mention in the docs might be in order?

Also, maybe there is a chance that Derby might want to remove those listeners from scoped models when the current page is being left? Not sure but that's what I thought it would do.

[SLaks]

Listeners on scoped models should work identically to regular listeners, except that they will be filtered to events within that part of the model.
They are not tied to the scoped model instance and are never automatically removed.
See https://github.com/codeparty/racer/blob/master/lib/Model.js#L254-L261

[SLaks]

removeAllListeners() is part of Node's standard EventEmitter API and will break Racer / Derby.

[ile]

They are not tied to the scoped model instance and are never automatically removed.

Well yes, that's why they caused trouble.

If a scoped model gets "out of scope" or "deleted" (not accessible anymore), I would have expected the listener to be removed. I would have expected the whole scoped model to be deleted, so that everything in it will be deleted as well.

Now the current behavior may lead to these similar issues with other users.

removeAllListeners() is part of Node's standard EventEmitter API and will break Racer / Derby.

I put this in the first message in this thread:

Models inherit from the standard Node.js EventEmitter, and they support the same methods: on, once, removeListener, emit, etc.

... so I expected I could use removeAllListeners()

[ile]

Also another thing to consider - if the scoped model "hangs around" .... is there already a memory leak somewhere in Derby because of that? Just a thought.

(Well, maybe not if there really isn't any scoped model objects anywhere, but the listeners are attached to some Derby's internal structures ...)

[SLaks]

A scoped model is just a reference to a particular piece of the model.
All of the data and event handlers still live on the original model. (in the implementation, scoped models simply inherit the original model, with an _at property that all model methods check for).

The only purpose of a scoped model is to allow you to use shorter paths (or to have a function that can work with multiple parts of the model by taking a scoped model)

I meant that removeAllListeners() is inherited "by accident", along with the rest of the prototype. Racer doesn't actually have any support for it.

[ile]

Yes. But still my point is that other people will have these same issues in the future. I think they will expect that listeners are removed.

So maybe something needs to be done. A clarification in the docs.

Also this should probably be changed if Racer actually doesn't support all of these:

Models inherit from the standard Node.js EventEmitter, and they support the same methods: on, once, removeListener, emit, etc.

[nateps]

Agreed there should be some clarification on the model events API in the docs, and the implementation should probably be updated to support listener removal better.

• Nate

On Thu, Mar 21, 2013 at 10:24 AM, Ilkka Huotari notifications@github.comwrote:

Yes. But still my point is that other people will have these same issues in the future. I think they will expect that listeners are removed.

So maybe something needs to be done. A clarification in the docs.

Also this should probably be changed if Racer actually doesn't support all of these:

Models inherit from the standard Node.js EventEmitter, and they support the same methods: on, once, removeListener, emit, etc.

—

Reply to this email directly or view it on GitHubhttps://github.com/codeparty/racer/issues/107#issuecomment-15252859 .

[nateps]

No longer relevant in 0.5