jashkenas/backbone — pull request #2836 by ragaskar (2013-10-29 14:58:33+00:00)
Title: Correct example of using a factory function for Collection#model

Collection#set expects the idAttribute to be present on model.prototype. If a function is provided here the user must manually add this property.

Without this property, collections w/ factory models fail to correctly update (and index in _byId) model idAttributes when fetching.

I have a collection with a model factory vs. reference to a Backbone Model.
Had a problem where after a re-fetch this collection can no longer fetch by model id.

If the models coming into 'set' are *not* backbone models, backbone will pull the id from the attrs by using the model.prototype.idAttribute, which my factory function -- being a plain old function and not a backbone model, doesn't have. This happens in the following code (around line 678, in Collection#set)

for (i = 0, l = models.length; i < l; i++) {
attrs = models[i];
if (attrs instanceof Model) {
id = model = attrs;
} else {
id = attrs[targetModel.prototype.idAttribute];
}

This is not a problem on the first add, because well, the model doesn't exist yet, so the comparison fails and all is well. On the second add, however, the existing isn't found, and so it goes into the wrong branch.

A very subtle bug, as the collection length still looks correct, since backbone thinks these are new objects. However, if you fetch by cid/id, you won't get the correct behavior.

I'm pull-requesting a docfix because it there's no small change that would let Backbone accomodate this (you certainly wouldn't want to *new* the model just to see if it had an idAttribute on it). Maybe if there was a collection-level idAttribute option that would make the most sense, but model factories seems like enough of an edge case that it's not a big deal.

The docs here aren't beautiful -- in my app, I threw my factory inside a anonymous scope so it isn't leaking into the global namespace, but this seems inconsistent with the Backbone doc style -- but hopefully enough so that other people won't hit the same bug.