`_reset` is called before Collection Constructor #9

Open
Crisfole opened this Issue Jan 31, 2013 · 7 comments

Comments

Projects
None yet
3 participants

It may be a documentation issue, or it may be a bug, I can't tell.

Line 59 results in an error because this.models (usually set in the Backbone.Collection constructor) hasn't been set yet. I'm not 100% sure what solution to suggest (thus why it's not a pull request) since I'm not really sure if it's a bug or documentation issue. Either way it's worth fixing because this is a very handy, well written tool.

Steps to Reproduce:

coll = new Backbone.Collection([], {})
new Backbone.Subset([], {parent: coll, sieve: function(m) { return true; }})

This happens whether or not you pass in the undocumented no_reset option. If no_reset is true the error fires at line 152

I've notice this too and have been trying to work on a fix, but haven't had time to properly test for it yet. @Crisfole What version of backbone are you using? I believe this breakage occurs after moving to 0.9.9 or 0.9.10.

Ah, interesting. I am using 0.9.10

Yup. That's it. Subset isn't yet compatible with 0.9.10.

On Thu, Jan 31, 2013 at 9:59 AM, Christopher Pfohl <notifications@github.com

wrote:

Ah, interesting. I am using 0.9.10


Reply to this email directly or view it on GitHubhttps://github.com/masylum/Backbone.Subset/issues/9#issuecomment-12955730.

Ah, and here's why: (from Backbone 0.9.9)

_reset: function() {
  this.length = 0;
  this.models = [];
  this._byId  = {};
  this._byCid = {};
},

has changed to (from Backbone 0.9.10)

_reset: function() {
  this.length = 0;
  this.models.length = 0;
  this._byId  = {};
},

Looks like adding this.model = []; helps.
Side note, doing:

if (options.sieve) {
  this.sieve = options.sieve;
}

is a really nice touch too...

Yeah, that may get things more, or less, working. But even with these fixes, many of the tests still break. Before going too deep into it, @masylum did you already have plans of upgrading Subset to work with backbone >= 0.9.10?

Owner

masylum commented Feb 7, 2013

Not yet... We are stucked at backbone 0.5.* at Teambox but we are trying to update ASAP. I will try to look into that issue, but if you provide a PR it would help a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment