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

Memory leak #1

Closed GianlucaGuarini closed 9 years ago

GianlucaGuarini commented 9 years ago

Anytime a todo entry gets removed the following event does not get removed with it:

RiotControl.on('todos_changed', function(items) {
    self.items = items
    self.update()
})

I have created a simple demo to demonstrate this http://jsfiddle.net/gianlucaguarini/kmLr3hbj/ Just click on 'add' and 'remove' to see your browser heap snapshots growing screen shot 2015-01-28 at 09 30 36

jimsparkman commented 9 years ago

RiotControl.on feeds into riot.on. You've traced it to eventing? If I remove all eventing, RiotControl and riot's observable, and just add remove 500 items from the tag's this.items the heap grows regardless.

jimsparkman commented 9 years ago

Also: https://github.com/muut/riotjs/issues/248

rsbondi commented 9 years ago

I have tested the multi todo example following the approach in #248 and I get the same thing, all objects persist so it appears to be with riot and not RiotControl

jimsparkman commented 9 years ago

Looks like @rsbondi is working on this.

https://github.com/muut/riotjs/pull/260

jimsparkman commented 9 years ago

Closing: The issue is being worked on in riotjs.

GianlucaGuarini commented 9 years ago

I am sorry but the problem is still there even with the newest riot release, you just need to run the demo http://jsfiddle.net/gianlucaguarini/kmLr3hbj/ already linked

screen shot 2015-02-09 at 23 30 52

tipiirai commented 9 years ago

Can you post this issue to Riot repository: https://github.com/muut/riotjs/issues

Thank you!

tipiirai commented 9 years ago

Ignore above! It's already there: https://github.com/muut/riotjs/issues/248