Improper direct call to Model's parse without an instance

[philfreo]

This line, from findOrCreate seems inappropriate.

var parsedAttributes = (_.isObject( attributes ) && this.prototype.parse) ? this.prototype.parse( attributes ) : attributes;

It caused a confusing bug for me because my Model#parse method expects its this context to be an instance of the Model, which it is 99% of the time. But when called this way, this is actually the model constructor, rather than an instance.

With the new { parse: true } option as of Backbone 0.9.9 where parse can be automatically called in a model's constructor, do we need this hack?

[DouweM]

The rationale behind the line is described in commit 90ab97c:

In the findOrCreate method, the idAttribute must be retrieved and it uses item[ type.prototype.idAttribute ]. This is fine when the API returns the id as a top level key but it brakes [sic] otherwise and parse must be called first.

That makes sense to me, so the attributes should definitely be parsed before calling Store.find(). We could still use the parse option in Model.build(), but that'd result in the attributes being parsed twice. It would, however resolve your issues around the context the method is called with. You could also provide the parse: true option to .findOrCreate() or methods delegating to it manually, would that solve your problems?

[philfreo]

You could also provide the parse: true option to .findOrCreate() or methods delegating to it manually, would that solve your problems?

I already am, but this doesn't stop parse from being called in a non-instance-context first (before it's called again properly in the build step.

In the findOrCreate method, the idAttribute must be retrieved and it uses item[ type.prototype.idAttribute ]. This is fine when the API returns the id as a top level key but it brakes [sic] otherwise and parse must be called first.

So the only reason parse is called like this is to Store.find() it properly if the ID attribute needs to be parsed first? What's an example where it couldn't just lookup idAttribute directly? (Isn't that the point of idAttribute being there, rather than hard coded as id?)

[DouweM]

The commit's author mentions:

This is fine when the API returns the id as a top level key but it brakes [sic] otherwise and parse must be called first.

So the attributes are something like this:

{
  person: {
    id: 123,
    name: "Douwe"
  }
}

And the parse() method would look something like this:

parse: function(attributes) {
  return attributes.person;
}

The problem is not with idAttribute.

[philfreo]

Ok - I don't really have a solution (aside from changing this functionality based on a new option), and it's easy enough to workaround in parse, just wanted to point out that calling an instance method outside of an instance context isn't normal in Backbone, and can lead to confusion. Feel free to close if you think everything is fine.

[DouweM]

I definitely see that the context not being the instance could be problematic, but since I don't see a better solution right now, I'm closing. Thanks for raising the issue though!

[philfreo]

For reference, there was discussion on Backbone itself about a similar issue. See jashkenas/backbone#2095 and jashkenas/backbone#2096