Ember.computed.filter replaces items in place when a property changes

[tim-evans]

I have a list of text fields bound to a filter, and we have a UX issue where when the field is auto-saved, the text field loses focus. I've tracked down the issue to reduceComputed properties. When flushProperties is called, it removes the item, then adds the item back.

This issue wasn't happening before I upgraded to 1.4.0. This might also be related to how quickly content gets flushed to the DOM. (The add and remove occur directly after each other- with the item in the same location. Should this affect the DOM?).

[wagenet]

@tim-evans are you able to provide a reduced test case?

[tim-evans]

@wagenet this seems to be more of a cause and affect thing using Ember Data and reduceComputed. When the data property is set on a model after a save, all properties on that model are marked as dirty (when some of them may not have changed). When the reduceComputed sees that the property has been changed, it flushes the changes, which rerenders the view.

It seems to me that there's no bugs in either implementation; there's just a side effect of how they work that causes this issue.

tldr; no, I couldn't provide a reduced test case.

[wagenet]

This still seems like a bug me to me. If an item is in the array both before and after it probably shouldn't be removed in the middle.

[wagenet]

@hjdivad thoughts?

[hjdivad]

@wagenet the problem is exactly as @tim-evans described. The issue is twofold:

1. ember-data invalidates data which is a dependency of all attributes
2. although observers are not triggered when properties are set to existing values, they are triggered when dependencies and chains are changed, even when this doesn't result in changes to the actual value.

Fixing #1 is doable I think. @stefanpenner IIRC we talked about this before; having attribute foo depend on data.foo rather than calling propertyDidChange('data') or something vaguely like that.

Fixing #2 is probably a good idea but a bit of work.

We could make reduceComputed ignore values that are the same as previous values but I worry about their semantics diverging too much from regular CPs.

As an interim solution we could do something a bit hacky in filter by not removing items whose values had not changed and then, by depending on the upstream reduceComputed implementation, knowing to ignore exactly one additional insertion. This seems like a reasonable and localised workaround for now. @wagenet @stefanpenner what do you think?

[sandstrom]

I think a related issue occurs when a Em.computed.filter that depends on isNew. When a model state changes to dirty a change is also triggered on isNew, which causes the filter to re-computed and all view-elements to be re-drawn.

I.e. someone types something in an input field, and it immediately loses focus.

App.MyController = Em.ArrayController.extend({
  persisted: Em.computed.filter('@this.@each.{isNew,isDeleted}', function(myObj) {
    return !myObj.get('isNew') && !myObj.get('isDeleted');
  })
});

Step-by-step explanation of the issue:
https://gist.github.com/sandstrom/1f3378d3927d6286aa27

[wagenet]

See #5268

[fishermand46]

I'm having the exact same problem as @sandstrom. Is there a work-around for this in a meantime?

[ahacking]

This sounds related to #9313 I have a very simple jsBin on that issue that demonstrates how a property change causes the affected item to be removed and then the entire array recomputed.

[fishermand46]

@hjdivad, I'm very interested the second scenario you mentioned earlier. Could you give an example of this happening? Is @ahacking's JSBin an example of this scenario? I just want to make sure I understand how this would happen as it seems like something that could bite you in the ass. :P

although observers are not triggered when properties are set to existing values, they are triggered when dependencies and chains are changed, even when this doesn't result in changes to the actual value.
@hjdivad