search

gmac / backbone.epoxy

Declarative data binding and computed models for Backbone
http://epoxyjs.org
MIT License
615 stars 89 forks source link

Fixed parameters for `change` event on computeds. #72

Closed Prinzhorn closed 9 years ago

Prinzhorn commented 10 years ago

Took me some time to figure out why my code behaved so weird...

A change event gets (model, options) and change:* gets (model, value, options). In its current state epoxy passes (model, value, options) to change as well.

Please note: I updated the code inside modifyArray and modifyObject as well, even though I don't know what they do (they are not used at all).

"change" (model, options) — when a model's attributes have changed. "change:[attribute]" (model, value, options) — when a specific attribute has been updated.

http://backbonejs.org/#Events-catalog

Edit: Sorry for the messy diff, but don't blame me that white space at the end of lines wasn't trimmed before.

gmac commented 10 years ago

@Prinzhorn Ha – touché on the messy whitespace. I'll have a look when I get a minute. FYI – modifyArray and modifyObject are developer-facing utility methods for performing updates to data structures (arrays and objects) and having them trigger events. While those methods aren't used by Epoxy internals, they're offered as a convenience to developers for modifying nested data structures within a model.

Prinzhorn commented 10 years ago

If your leave modifyArray and modifyObject aside, I only changed a single line. I'll add a comment to it to make it easier for you.

Prinzhorn commented 10 years ago

I'm still facing issues and I think my fix doesn't include all cases. There is one change case which I'm not sure how to fix it (see https://github.com/gmac/backbone.epoxy/blob/e6e96224c8d451df97087f2bdb73cedf6234a6e4/backbone.epoxy.js#L138).

Here's so background so you understand what's going on: I'm syncing models across multiple pages using postMessage. This almost works like a charm, but the change events of the computeds ruin everything. When a page receives a message with updated model data, I use model.set(event.changes, {local: true}). The important part is the local: true option which prevents endless sync loops because these changes won't be dispatched to the other pages again. What I need epoxy to do is to correctly pass along the options when trigger change events on computeds. This also includes what I did in this pull request, because the parameters for change:* where wrong and thus the options as well.

gmac commented 9 years ago

Closing this out for now as it sounds like this is still a working feature, and may relate to some work that's been done on the library in the past year. If this is still relevant, please issue a fresh pull request with a current writeup.