gmac / backbone.epoxy

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

Epoxy fires 'change' before all computeds are re-calculated #138

Open burgalon opened 8 years ago

burgalon commented 8 years ago

This is a result of the change() code Line 486:

evt[0] += ' change';

Can we remove this?

Currently when listening to to 'change' event you might want to do some logic that is related to a computed value, however the computed value is still not re-calculated, containing stale value.

jekuno commented 8 years ago

If you remove this line no generic change event (but only a change:computedAttribute) will be triggered when updating the computed using get(true). See https://github.com/gmac/backbone.epoxy/blob/master/backbone.epoxy.js#L455

emileber commented 8 years ago

But it never make sense to trigger both change:computedAttribute and change events at the same time since the callbacks take different arguments. And in certain cases (see #140) the generic change event is triggered twice.

jekuno commented 8 years ago

I understand your point and myself stumpled upon change events being triggers twice. However by simply removing https://github.com/gmac/backbone.epoxy/blob/master/backbone.epoxy.js#L486 you will not have any generic change event triggered when updating the computed using get(true). This means you might miss a change if you only listen to the generic change event. Or am I missing something here?

emileber commented 8 years ago

Could you explain when you would update the computed using get(true)? Maybe provide a use case?

jekuno commented 8 years ago

As denoted in https://github.com/gmac/backbone.epoxy/issues/100 you cannot add a collection as a dependency of a computed attribute. To update computeds on collection changes you have to listen to those collection changes and recompute the according computeds manually.

We can remove line https://github.com/gmac/backbone.epoxy/blob/master/backbone.epoxy.js#L486. But then we have to keep in mind that we have to trigger a generic change event manually when recomputing a computed attribute manually. You could achieve this every time you recompute an attribute by using a wrapper function.

Or am I missing anything?