Skip to content

Commit

Permalink
Fix recursion error when saving from within a change event.
Browse files Browse the repository at this point in the history
  • Loading branch information
Matt committed Nov 23, 2010
1 parent 9e1fcfa commit 57194be
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
3 changes: 1 addition & 2 deletions backbone.js
Expand Up @@ -242,9 +242,8 @@
// If the server returns an attributes hash that differs, the model's
// state will be `set` again.
save : function(attrs, options) {
attrs || (attrs = {});
options || (options = {});
if (!this.set(attrs, options)) return false;
if (attrs && !this.set(attrs, options)) return false;
var model = this;
var success = function(resp) {
if (!model.set(model.parse(resp), options)) return false;
Expand Down
9 changes: 9 additions & 0 deletions test/model.js
Expand Up @@ -151,6 +151,15 @@ $(document).ready(function() {
model.change();
equals(model.get('name'), 'Rob');
});

test("Model: save within change event", function () {
var model = new Backbone.Model({firstName : "Taylor", lastName: "Swift"});
model.bind('change', function () {
model.save();
ok(_.isEqual(lastRequest[1], model));
});
model.set({lastName: 'Hicks'});
});

test("Model: save", function() {
doc.save({title : "Henry V"});
Expand Down

2 comments on commit 57194be

@KrisJordan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit seems to cause other scenarios to regress. Specifically, because set calls validate and set is getting short wired when attrs is false, validate never runs on saves to Models instantiated with attributes. Because Collection.create's implementation relies on that scenario validation does not get triggered when Models are created using Collection.create even though the documentation implies otherwise (http://documentcloud.github.com/backbone/#Collection-create).

Examples here: https://gist.github.com/813138

On further inspection of set it now shortwire returns this with empty attrs before running validate so getting validate to run on Collection.create may just need a localized fix there.

@jashkenas
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix in your pull request for Collection#create ... which has been merged to master.

Please sign in to comment.