davidgtonge / backbone_query

A lightweight query api for Backbone Collections
MIT License
378 stars 29 forks source link

whereBy is making the _events array leak #16

Closed pocesar closed 6 years ago

pocesar commented 11 years ago

the line on whereBy:

return new this.constructor(this.query(params, options));

is adding to the model _events.all, many callbacks, and the array grows until the app start to feel laggy or unresponsive. I'm not sure what would the fix be. I'm using {cache: true}, but that doesn't seem to make a difference.

Here is the reproducible test case

http://jsfiddle.net/7pfCf/15/

Toggle the items, and press the button. Each time the button is pressed, a whereBy query is performed, and another item is appended to each model _events array. So, right now, I'm having to use underscore functions instead of chaining using whereBy

davidgtonge commented 11 years ago

Thanks for reporting this, I've nearly finished quite a big refactor which will help with this issue

pocesar commented 11 years ago

nice, because this library is really nice, thanks for your work

davidgtonge commented 11 years ago

An initial thought, is that this behaviour probably can't be fixed by my library. Essentially whereBy is creating a new collection every time it is called, and all the models from the "parent" collection that pass the query are added to this new collection. If you no longer need this new collection you would need to remove all the models from it, otherwise the models will still have a reference to that collection.

For the next version of the library I've added in the ability to have live collections - this will be a much better way of working than the current whereBy implementation.

pocesar commented 11 years ago

there could make use of the chain() "native" method of Backbone.Collection, the only drawback is the need to call .value() at the end of the chain to execute it. like so:

whereBy: function(query){
  return _(this.query(query)).chain();
}

The problem is that it doesn't return a new collection, but an object to deal with the result array of query() call.

Looking forward for the live collection :)

pocesar commented 11 years ago

just a quick note, it's working fine with Backbone 1.0

davidgtonge commented 11 years ago

I've been a bit snowed under at work. But am nearly ready to push the updated version. The API is much improved imho

pocesar commented 11 years ago

good to know! :+1: