Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If set() includes an id, the id gets set as an attribute of the model. #132

Closed
HenrikJoreteg opened this issue Dec 9, 2010 · 3 comments
Closed

Comments

@HenrikJoreteg
Copy link
Contributor

The title says it all. This is also a problem because it can incorrectly trigger a 'change' event.

@jashkenas
Copy link
Owner

This is how it's supposed to behave. You're notified when a model you're observing changes id, and can even listen for "change:id" events. Having the id be treated specially and being set as a direct property of the model object is a convenience:

http://documentcloud.github.com/backbone/#Model-id

Is there a particular reason you think it should behave differently?

@HenrikJoreteg
Copy link
Contributor Author

So, after looking into it more, that's not the problem. I have no problem with id behaving that way. When I got an unexpected change event and then saw the id listed in attributes I assumed it was a mistake. But, I just hadn't noticed before.

The real problem is that this test fails:

test("Model: change event bug", function () {
  var changed = 0;
  var attrs = {id: 1, label: 'c'};
  var obj = new Backbone.Model(attrs);
  obj.bind('change', function() { changed += 1; });
  obj.set(attrs);
  equals(changed, 0);
});

Nothing should have changed — yet, I got a change event. Also, if inside the change callback you call changedAttributes() you get false.

This sure seems like a bug, no?

@jashkenas
Copy link
Owner

It sure is -- thanks for reporting this. Here's the patch with a fix:

53ae5b5

deleteme pushed a commit to deleteme/backbone that referenced this issue Jun 1, 2011
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants