Collection.parse should be called in .add if options.parse is true

[davidmarble]

In the constructor of Collection (currently line 554 - https://github.com/documentcloud/backbone/blob/master/backbone.js#L554), the below line indicates that the reset method, or add method called by reset, should check options.parse, and if if true, call parse on the models passed in.

if (models) this.reset(models, {silent: true, parse: options.parse});

However, there is no such logic in the add or reset methods of Collection. Seems to be an oversight.

[braddunbar]

Hi @davidmarble, thanks for pointing this out. While there is no explicit call to parse in add and reset, they both call the collection's model constructor with the options they're passed, which may or may not include parse. For instance, the following will log "parse":

var Model = Backbone.Model.extend({

  parse: function(resp) {
    console.log('parse');
    return resp;
  }

});

var Collection = Backbone.Collection.extend({model: Model});
new Collection().reset([{id: 1}], {parse: true});

[davidmarble]

@braddunbar, yep, I get that parse() will be called on the models themselves because of _prepareModel.

I opened the issue because there's inconsistency in how parse() is called on models and collections. This issue is about Collection.parse(), not Model.parse().

The inconsistency is that parse() is only called in Collection.fetch(), while there are several other ways a developer may want to populate a Collection from not-yet-preprocessed javascript arrays/objects.

3 ways I can think of:

1. passing an array of objects to the Collection constructor, which calls reset()
2. calling reset() directly
3. calling add() with an object instead of an initialized Model

Developers can sort of work around these by calling collection.parse() on objects for all of the above cases before they send them to those methods, but I'm just pointing out that this is rather inconsistent -- the functionality is only automatically applied in fetch().

[braddunbar]

Ah! I misunderstood. You mean that you should be able to do collection.reset({models: [...]}, {parse: true}) and have collection.parse called on models.

I haven't thought about it before, but it does seem inconsistent.

[DylanArnold]

I just came up against this issue too and assumed that passing parse: true to a collection in these ways should mean that parse on the collection will get executed. The problem is that this is executed when calling fetch, but when I initially load my page I am constructing collections with the same JSON and parse isn't called. I guess I'll have to call it manually until/if this is fixed.

[jashkenas]

This should be working now on master.