Inconsistent Parsing on Model.fetch and Collection.fetch

[marcusmacinnes]

The Collection's fetch method correctly sets a default value for options.parse=true which allows for the collection's models to be parsed.

// From Backbone.Collection

// Fetch the default set of models for this collection, resetting the
// collection when they arrive. If `add: true` is passed, appends the
// models to the collection instead of resetting.
fetch: function(options) {
  options = options ? _.clone(options) : {};
  if (options.parse === void 0) options.parse = true;
  var collection = this;
  var success = options.success;
  options.success = function(resp, status, xhr) {
    collection[options.add ? 'add' : 'reset'](collection.parse(resp, xhr), options);
    if (success) success(collection, resp, options);
  };
  return this.sync('read', this, options);
},

The Model's fetch method doesn't set the default value for options.parse=true which prohibits the default parsing of nested models and collections.

// From Backbone.Model

// Fetch the model from the server. If the server's representation of the
// model differs from its current attributes, they will be overriden,
// triggering a `"change"` event.
fetch: function(options) {
  options = options ? _.clone(options) : {};
  var model = this;
  var success = options.success;
  options.success = function(resp, status, xhr) {
    if (!model.set(model.parse(resp, xhr), options)) return false;
    if (success) success(model, resp, options);
  };
  return this.sync('read', this, options);
},

Can we bring the behavior of these two methods into sync by adding the following line:

if (options.parse === void 0) options.parse = true;

to the model's fetch method as follows:

// Fetch the model from the server. If the server's representation of the
// model differs from its current attributes, they will be overriden,
// triggering a `"change"` event.
fetch: function(options) {
  options = options ? _.clone(options) : {};
  if (options.parse === void 0) options.parse = true;
  var model = this;
  var success = options.success;
  options.success = function(resp, status, xhr) {
    if (!model.set(model.parse(resp, xhr), options)) return false;
    if (success) success(model, resp, options);
  };
  return this.sync('read', this, options);
},

This will enable consistent parsing behavior and facilitate the ability to parse nested models and collections.

[jashkenas]

@braddunbar -- any thoughts on this one?

[braddunbar]

:+1: I think parsing by default is a good thing.

[dgbeck]

me too, seems like I am always having to do { parse : true }

[jashkenas]

I'm a bit confused now about the current behavior. Wouldn't this be better:

• Never parse by default.
• Always parse if a parse function is defined.

[braddunbar]

Yep, that sounds great to me.

[dgbeck]

I like it!

[caseywebdev]

@jashkenas I raised a concern in the commit https://github.com/documentcloud/backbone/commit/fbbb2e675e594f023ca17cf3708bfb07601e8ff2#commitcomment-2221749

[marcusmacinnes]

@jashkenas The problem still exists.. Can I re-open this issue?

Scenario: When the server responds to a fetch with a deep data document (containing data for nested models and collections).

The fetching "root" model should parse it's part of the "data", but that model shouldn't know anything about the child model's parsing requirements. The root model should be be able to pass the parse flag down the chain so that child models which are not yet instantiated can perform "their" parse of "their" part of the data.

It's important therefore be able to push this parse flag down the chain and this can only be done by setting a default options.parse = true with:

if (options.parse === void 0) options.parse = true;

as indicated in the code original post above...

Any thoughts?

[caseywebdev]

@marcusmacinnes It's in master now :+1:

[marcusmacinnes]

Perfect - thanks