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.

Sign up for GitHub

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

Jump to bottom

Setting hashes with Mutators breaks Backbone atomicity #26

Open
dsaffy opened this issue Aug 27, 2013 · 1 comment
Open

Setting hashes with Mutators breaks Backbone atomicity #26

dsaffy opened this issue Aug 27, 2013 · 1 comment
Assignees
@asciidisco
Labels
bug

Comments

@dsaffy
Copy link

dsaffy commented Aug 27, 2013

Consider model.set({key1: val1, key2: val2});

In traditional Backbone, this will set both keys to their respective values and then fire change:key1 and change:key2 events. In other words, Backbone guarantees the atomicity of set calls with hashes. Mutators breaks this contract by firing each change event after it's value is set. So Mutators would set key1 to val1, then fire the change:key1 event, then set key2 to val2, then fire the change:key2 event.

I believe this is because of the way that calls to oldSet works in the Mutators source.
@ghost ghost assigned asciidisco Oct 30, 2013
cluebcke pushed a commit to cluebcke/Backbone.Model.Plus that referenced this issue Apr 9, 2014
Rewrote set method to incorporate Backbone's, rather than to call it in
6c1a984 
a 'super-y' way. This is because calling Backbone's set at the beginning
of the set method causes Backbone to immediately emit change events,
before any mutators or nested properties are set.

Also, calling Backbone's set meant that every set of a mutator actually
created a property with that name in the attributes hash. This was
unexpected and occasionally problematic.

The new set now only adds attributes that the user specifically expected.
It also only emits change events after any mutators have been run and
nested properties set. (This fixes
asciidisco#26).
@asciidisco
Copy link
Owner

@dsaffy I do not really have the time to support this project anymore, if you can get in a PR, I would be happy to merge it & to release a new version, thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@asciidisco asciidisco

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@asciidisco @dsaffy