Tracked properties accessed in Array.prototype.map and then updated don't retrigger render

[scottmessinger]

Hi!

I'm sorry in advance for the title -- it's a bit hard to describe. Here's what's going on:

Let's say you have this component/controller:

export default class ApplicationController extends Controller {
  
  @tracked
  course = {
    attributes: {
      calendar: { 
        dates: [
          {id: "monday", attributes: { lessonId: "a"} },
          {id: "tuesday", attributes: { lessonId: "a"} },
          {id: "wednesday", attributes: { lessonId: "a"} },
          {id: "thursday", attributes: { lessonId: "a"} },
          {id: "friday", attributes: { lessonId: "a"} }

        ]
      }
    }
  }
}

And then you have a getter which calculates something. E.g.:

  get countOfUniqueLessonIds(){
    return this.course.attributes.calendar.dates.map(date => {
       return date.attributes.lessonId
    }).uniq().length
  }

and render it to your template:

Unique Lessons: {{this.countOfUniqueLessonIds}}

And then you update a value deep in the object:

  @action makeMondayHaveUniqueLessonId(){
     set(this.course.attributes.calendar.dates[0].attributes, "lessonId", "b")
  }

The getter won't update, even though the value will be updated:

   Unique Lessons: 1 # should say 2

Here's a twiddle:
https://ember-twiddle.com/4598ec7b9b56b1d6db3acc841f3728f7?openFiles=controllers.application%5C.js%2C

To reproduce:

• Click the button
• Notice that Monday's lesson id is updated but the count of unique lesson ids hasn't changed.

I've been running into this for a few weeks with 3.17.x and took a minute to try to create a reproduction. Is this expected behavior? If so, how should we handle these situations in ember apps?

As a point of context, in pre-Octane apps, I'd add a computed property and it would work. To get around this, I'm just adding computed properties to my glimmer components. As it's a rather deep object, I hack it by having the CP look at property I update each change (@computed('internalVersion')) or I'll do a few computed properties due to limitations around @each.

Let me know if there's anything I can do to help if this isn't expected behavior!

[scottmessinger]

Also, I went back and added tracked versions of the objects and arrays using https://github.com/pzuraq/tracked-built-ins

  @tracked
  course = {
    attributes: new TrackedObject({
      calendar: new TrackedObject({ 
        dates: new TrackedArray([
          new TrackedObject({id: "monday", attributes: new TrackedObject({ lessonId: "a"}) }),
          new TrackedObject({id: "tuesday", attributes: new TrackedObject({ lessonId: "a"}) }),
          new TrackedObject({id: "wednesday", attributes: new TrackedObject({ lessonId: "a"}) }),
          new TrackedObject({id: "thursday", attributes: new TrackedObject({ lessonId: "a"}) }),
          new TrackedObject({id: "friday", attributes: new TrackedObject({ lessonId: "a"}) }),
        ])
      })
    })
  }

However, I got the same result :-( The value I calculated by mapping over the nested array did not update.

[pzuraq]

This is expected behavior, though I can see why it's confusing.

The issue is that if you want to update a piece of non-tracked state using Ember.set(), then you must always access it using Ember.get, even in your native getter:

  get countOfUniqueLessonIds(){
    return this.course.attributes.calendar.dates.map(date => {
       return get(date.attributes, 'lessonId');
    }).uniq().length
  }

Ember cannot know ahead of time that you plan to update lessonId, so you need to tell it "this value is tracked dynamically" by using Ember.get. Templates do use Ember.get, as to computed properties (effectively), so they will respond to Ember.set.

You should really only do this if your state is highly dynamic though, or if you're using classic patterns and trying to update the code. The modern solution would be to create a class that annotates the tracked properties:

class Attributes {
  @tracked lessonId;

  constructor(lessonId) {
    this.lessonId;
  }
}

export default class ApplicationController extends Controller {
  
  @tracked
  course = {
    attributes: {
      calendar: { 
        dates: [
          {id: "monday", attributes: new Attributes("a") },
          {id: "tuesday", attributes: new Attributes("a") },
          {id: "wednesday", attributes: new Attributes("a") },
          {id: "thursday", attributes: new Attributes("a") },
          {id: "friday", attributes: new Attributes("a") }

        ]
      }
    }
  }
}

If you're dealing with dynamic classes where you don't know the tracked keys ahead of time, or you find that making classes for every case is burdensome, and you don't need to support IE11, you could consider using tracked-built-ins:

import { tracked } from 'tracked-built-ins';

export default class ApplicationController extends Controller {
  
  @tracked
  course = {
    attributes: {
      calendar: { 
        dates: [
          {id: "monday", attributes: tracked({ lessonId: "a"}) },
          {id: "tuesday", attributes: tracked({ lessonId: "a"}) },
          {id: "wednesday", attributes: tracked({ lessonId: "a"}) },
          {id: "thursday", attributes: tracked({ lessonId: "a"}) },
          {id: "friday", attributes: tracked({ lessonId: "a"}) }
        ]
      }
    }
  }
}

[pzuraq]

hmm, if TrackedObject isn't working then something is wrong. Digging in.

[scottmessinger]

@pzuraq Thank you so, so, so much for the clear explanation! I can confirm accessing it with Ember.get works! That's awesome -- I'm really excited to hear this. We've used Ember.get for years, so continuing to do that isn't a big deal at all. Tracked is a pretty magical (in a good way) so needing to using Ember.get in these situations is totally fine.

Are there any perf reasons to do instead creating classes? That's an option we could look at (though would be more work) if we're going to be better off perf wise.

After reading your message, I went back to double check it wasn't working with new TrackedObject (or tracked) and I can confirm that still doesn't work. I've updated the twiddle.

[pzuraq]

ahhhh 🤦‍♂ this is yet another issue caused by the mandatory setter. We really need to prioritize removing this. Apparently the way it does defineProperty on TrackedObject messes things up when you use it directly in a template 🙃 See this issue for more details: #18769

I'll need to patch this upstream in TrackedObject somehow, don't want folks to continue to be bitten by it. Anyways, in the meantime, you can still use class definitions directly.

Perf-wise, there are some benefits to using class definitions. For instance, they ensure that the shape of your classes is always the same, which can have massive benefits in hot paths. I've heard that browsers are looking to optimize them further, but it's hard to say how much benefit that'll be. The tradeoff is bytesize - more classes does mean more to download.

If you are going to be enumerating a reasonably sized set of classes that are contained, and well defined, I recommend always using classes. This helps you in three ways:

1. It makes the shapes of objects you're working with super clear. No need to worry what keys an object has on it, it's documented in the class definition.
2. It helps normalize things shape-wise, which like I mentioned can be good for perf.
3. It gives you a place to mark @tracked props naturally.

If you're going to working with a lot of dynamic class definitions though, maybe ones that have various combinations that can be highly dynamic, I would not try to do this. It will be a huge pain to enumerate all of them, maintain them, and it will cost a large amount in terms of byte-size.

You can try to strike a balance between the two, as well. Usually, part of your data structure is well defined, and part of it is less-so. You can use a class for the well defined bits, and use TrackedObject and TrackedArray, etc. for the dynamic ones.

Hope this helps!

[scottmessinger]

@pzuraq That's SUPER helpful! Thank you so, so much! I can't thank you enough!

[pzuraq]

I'm going to close this issue and work to fix it upstream in TrackedObject somehow, since it's not directly related to Ember and we have an issue open for removing mandatory setter already.