Cannot observe changes to nested properties on array using @each

[robharper]

Adding an observer on a nested property of objects within a collection does not reliably fire. I've created a test case that could be added to ember-runtime/tests/mixins/array_test.js that illustrates this. Observing a nested property on the EachProxy itself appears to work fine. However, observing the property using @each.nest.isDone does not.

In the following test, count1 is incremented to 1, count2 is not:

test('modifying nested contents of an object in an array should call @each observers', function() {
  var ary = new TestArray([
      Em.Object.create({ nest: Em.Object.create({ isDone: false}) }),
      Em.Object.create({ nest: Em.Object.create({ isDone: false}) }),
      Em.Object.create({ nest: Em.Object.create({ isDone: false}) }),
      Em.Object.create({ nest: Em.Object.create({ isDone: false}) })
    ]);

  var get = Ember.get, set = Ember.set;
  var count1 = 0, count2 = 0;

  // Works
  var each = get(ary, '@each');
  Ember.addObserver(each, 'nest.isDone', function() { count1++; });

  // Doesn't work
  Ember.addObserver(ary, '@each.nest.isDone', function() { count2++; });

  count1 = count2 = 0;
  var item = ary.objectAt(2);
  get(item, 'nest').set('isDone', true);

  equal(count1, 1, '@each.nest.isDone should have notified - observing @each array');
  equal(count2, 1, '@each.nest.isDone should have notified - observing chain containing @each');
});

[alexspeller]

@rwwagner90 regarding observers, my take on it is that using observers in cases when you're not interacting with non-ember code is a big code smell. So using observers to e.g. trigger a d3 graph rendering when your app data changes is totally legit, but using observers to set one ember property when another changes is almost always more easily accomplished with a different approach

[RobbieTheWagner]

@alexspeller @kingpin2k I have two properties, and one must change when the other changes. This is not possible though because properties don't evaluate, so the second property never updates.

[alexspeller]

@rwwagner90 having one property that changes when another changes is a core feature of computed properties, so I'm not sure what you're describing - if you show the code you're using it's likely that someone can help.

[RobbieTheWagner]

@alexspeller I converted it all to using observes, but let me attempt to put it back and show you.

let extendedObject = Ember.Object.extend({
          valueLength: function() {
            return this.get('value').length;
          }.property('value.@each')
        });

Then I have a property that should update when valueLength does:

}.property('model.data.@each', 'model.data.@each.valueLength'),

However, this never fires, so I was forced to change my valueLength property to an observer that sets valueLength instead like so:

let extendedObject = Ember.Object.extend({
          valueLengthUpdater: function() {
            this.set('valueLength', this.get('value').length);
          }.observes('value.@each')
        });

[krisselden]

@rwwagner90 your dependency keys don't match what you are getting, your dependency key should be model.data.length

[krisselden]

Also, model.data.@each.valueLength is ok, by you should then actually get('valueLength') when computing a value. In a CP, generally speaking, if you are doing ending a dependency with just '@each' you aren't going to have a good time, and dependencies need to be consumed as part of the CP calculation. You cannot assume things about stuff changing together (stuff is invalidated synchronously and if there are observers that consume the property it can lead to race conditions), dependencies should align with what you actually consume as part of making the calculated value.

Generally, observers are low level, and hard to get right, you shouldn't actually do work in them (it can lead to race conditions, overly frequent recalculations, etc) they should only schedule work once per event frame.

[kingpin2k]

Ditto @krisselden. Also, I'm assuming your model on some controller has a property data which is a collection of extendedObject and in the controller you have the computed property, and your extendedObject has a length property on it (which sounds like it should be an array, and not an object, but ignoring that:

Controller

something: function(){
   // inside of here you should actually fetch `valueLength` off of each item, or else it isn't really a dependecy
  // assuming data returns an array
  var lengths = this.get('model.data').getEach('valueLength');
}.property('model.data.length', 'model.data.@each.valueLength'),

[alexspeller]

Edit:

You can replace valueLength property with valueLength: Ember.computed.alias('value.length').

Your dependent key then becomes:

}.property('model.data.@each.valueLength')

See @krisselden's explanation below on why what I wrote previously won't work.

@rwwagner90 valueLength is not necessary there - you can just use the value.length property. The property's dependent key can be simplified to:

}.property('model.data.@each.value.length')

@each shouldn't be used without a further property on the end. If you just want to observe when items are added or removed use somearray.[] instead. If you observe foo.@each.something then you don't need to also observer foo.[]

[krisselden]

@alexspeller Actually you can't

[krisselden]

@alexspeller get('@each.value') unfortunately returns an array of the values instead of returning an EachProxy which kills further chaining. I don't know why that was done that way but I don't know if I can change it given semver, and to be honest, I'm nervous about enabling this.

[kingpin2k]

The squeaky wheel gets the oil, until it's affecting a larger portion of the ember community it won't get prioritized up, there is plenty of other low hanging fruit that helps 90% of the community.

[RobbieTheWagner]

@alexspeller Ember.computed.alias worked. Thanks! :+1:

[RobbieTheWagner]

@krisselden Chaining @each would be awesome, but no one has tackled it in the past few years, so it doesn't seem likely that it ever will be.

[vothai-son]

I hope this issue is still on cause I don't know where to post this issue. I am using Ember 2.1.0+45f524a3

I got the observer FIRED with this:

var o = Ember.Object.create({
    a: [{ b: 1 }, {b: 2} ]
})

Ember.addObserver(o, 'a.@each.b', o, function () {
    console.log("FIRE")
});

Ember.set(o.a[0], 'b', 3)

But with this, nothing happens

var o = Ember.Object.create({
    A: [{ b: 1 }, {b: 2} ]
})

Ember.addObserver(o, 'A.@each.b', o, function () {
    console.log("FIRE")
});

Ember.set(o.A[0], 'b', 3)

(The difference is the case of property name "a" and "A")

So, anyone recognizes this issue, please help me explain. Thanks.

[alexspeller]

Unconsumed Computed Properties Do Not Trigger Observers

[vothai-son]

Thanks. But I have it triggering with the lowercase "a".

var o = Ember.Object.create({
    a: [{ b: 1 }, {b: 2} ]
})

Ember.addObserver(o, 'a.@each.b', o, function () {
    console.log("FIRE")
});

Ember.set(o.a[0], 'b', 3)

And no computed property here.

[alexspeller]

Oh, the upper case a is not supported. Use property names that start with lower case letters.

[vothai-son]

Thanks, This happens with an uppercase name and ".@each." It's fine with simple property. Like this:

var o = Ember.Object.create({
    A: 1
})

Ember.addObserver(o, 'A', o, function () {
    console.log("FIRE")
});

Ember.set(o, 'A', 3)

I can't find documentation for that anywhere. I think I will endup with a bunch of lowercase aliases for those uppercase named properties.

[rwjblue]

Historically (pre-2.0), capitalized paths indicated a global (so in your case it would be observing window.A). This issue is talking about nested paths with @each, which your comment is not discussing.

Most (hopefully all) of the special casing of capitalized paths should be removed. We should test against 2.8.1. If it still isnt working on 2.8, please open a new issue with a twiddle/jsbin reproduction.

[timothyerwin]

This is extremely basically functionality which is sadly missing. It would be helpful to focus on getting the basics right before going off adding all these bells and whistles to the framework.

[luxzeitlos]

@timothyerwin actually it's pretty easy to work around this. Also I think right now is a bad moment to change this. Things like '@tracked' im glimmer show, that we need to re-think the entire CP-thing in the long term. Also don't get me wrong, I would love to have this sometimes!

[lukemelia]

This is the Ember issue I encounter most frequently lately, often because it is hidden behind something else, like a filter or sort CP macro where it's easy to forget you effectively creating a dependent key that is an @each with nested properties.

[jacobq]

@luxferresum Would you mind posting a link to more information about how to work around this? Admittedly, I repeatedly get stumped by this, and I know I've been given some hints via community slack help channel, but I can't remember them nor can I seem to find a good post from searching the web. I think perhaps the trick was to break out each level as its own computed property somehow.

So if I have an array like traces: [{ x: [...], y: [...]}, ...], and I want to trigger an external charting library to update based on the (not allowed) key traces.@each.{x,y}.[] would I flatten it like this?

// Not only is this ugly, but it doesn't work since `_oneBigMess` won't get
// called again when something in one of its nested arrays changes,
// e.g. Changing traces[0].x[0] does not trigger 'traces.[]'
_oneBigMess: computed('traces.[]', function() {
  return this.get('traces').reduce(function(result, item, i, a) {
    return {
      x: result.x.concat(item.x),
      y: result.x.concat(item.y)
    };
  }, { x: [], y: []})
}),
_somethingChanged: observer('_oneBigMess.x.[]', '_oneBigMess.y.[]', function() {
  // update chart
})

I'm sure the above code is idiotic to someone more experienced and familiar with the tools available, but I am grasping at straws here.

• Old SO post here
• GH comment here
• Discuss forum

but none seem to have a work-around for the situation I described 😢

Update: Someone please correct me if I'm wrong, but after fighting with this for a while tonight, it seems that the problem here isn't merely about providing more syntactic sugar but really about a limitation of the current system. If I understand correctly (based on the old comment above and some search results) this functionality isn't provided yet because there is not an efficient / clean way to do it. The advice that I was given was basically to not do this but rather change the design of my code so that I didn't need to. This is probably good advice but sometimes may not be feasible (e.g. insufficient time or requirements outside of one's control). For two-levels, one (pretty horrible & inefficient) work-around is to generate a long (maximum data size your code needs to handle correctly) list of keys, each associated with a single element of the array. For example:

_watchChanges: observer('data.[]', 'data.@each.{name,type}',
  ...(new Array(100).fill(0).map((z,i) => `data.${i}.x.[]`)),
  ...(new Array(100).fill(0).map((z,i) => `data.${i}.y.[]`)), function() {
  // something in one of these arrays has changed --> go do something
})

Another possibility (though I haven't tried it, and it's still not ideal) would be to iterate over the top-level array and use addArrayObserver to watch each nested array (and then removeArrayObserver later to avoid leaking).

[luxzeitlos]

@jacobq

but rather change the design of my code

true. however there are much easer ways to handle this than you suggest. let me show it to your by your original example, with an array traces: [{ x: [...], y: [...]}, ...], where you basically want to get a list of all x and all y.

the primary trick/workaround here is to convert each trace into an ember object, and then have a CP on the Trace with a dependency key of x.[] and y.[]. This CP will fire whenever something is added/removed in the x/y arrays. Next you can provide a simple CP that has traces.@each.myProp as a dependency key, where traces is the list of ember Trace objects, and myProp is the CP I just talked about.

Or with code:

const Trace = Ember.Object.extend({
  wrapped: Ember.computed('x.[]', 'y.[]', {
    get() {
      return {
        x: this.get('x'),
        y: this.get('y'),
      }
    }
  }),
});

export default Ember.Component.extend({
  traces: null,
  tracesAsEmberObjects('traces.[]', {
    get() {
      this.get('traces').map(t => Trace.create(t));
    }
  }),
  oneBigMess: Ember.computed('tracesAsEmberObjects.@each.wrapped', {
    get() {
      return this.get('tracesAsEmberObjects').reduce((result, item, i, a) => {
        return {
          x: result.x.concat(item.x),
          y: result.x.concat(item.y)
        };
      }, {x:[], y:[]});
    }
  }),
});

actually it doesnt really matter what wrapped returns, as long it has the right dependency keys, you .get() it, and have it as a .@each.wrapped dependency key in the outer CP. However I prefer it to return something useful.

[jacobq]

@luxferresum Thanks for taking the time to reply with a potential solution. However, it seems to have the same problem. Although creating a new EmberObject with a computed property allows one to react to changes of the arrays held by that object, the process of creating that object seems to create a copy of the original data, so changing the original data does not trigger the CP/observer.

For example, in the ember-cli-plotly addon I'm trying to make I have an integration test that changes the data to be plotted, and it works when I use a kludgy work-around where the main component generates a tagless child component for each trace, which then fire an action when data in their set changes.

I will try your method a little more in case I'm just doing something wrong (very possible), but the way it's acting now makes me think that it's not as simple as you suggest.