Angular.js: ngOptions "track by" makes copies of array elements, expected references

[p0lar-bear]

I'm submitting a ...

• [x] bug report
• [ ] feature request
• [ ] other (Please do not submit support requests here (see above))

Current behavior:
Use of the track by clause on a <select> using ngModel creates copies of elements/propertied from the source array/object. Changes to the ngModel value are not reflected anywhere.

Expected / new behavior:
I expected changes to the ngModel value to propagate back to the source array/object.

Minimal reproduction of the problem with instructions:
https://embed.plnkr.co/ifB5nJ9wp8HUxzsSAi9G/

Select items from the list and attempt to edit the values in the box. No changes will take.

Angular version: 1.6.4

Browser: Chrome 58, IE 11

Anything else:
This is unexpected behavior to me. The documentation for both ngOptions and ngRepeat make no mention of this.

To accomplish a use-case like in the above-linked Plunker, one would currently need to write a lookup algorithm to find the model in the source array, and then apply the changes. Personally, I don't see what the concern is over the application mutating the selected object - I wouldn't think Angular developers would let even privileged users of a webapp modify the unique identifier of an object in a collection, instead let the XHR calls to the server handle creating unique IDs. But, I may have a limited perception here.

I feel this either needs to be documented, or it shouldn't be calling angular.copy on selected items when track by is in use.

[gkalpak]

A little background on how we got here:

1. This behavior of copying elements was introduced in 6a03ca274 (with a long commit messge explaining why). At this point, we had a deep $watch to catch modifications of the selected object.

2. Then in 47f9fc3e7, the deep $watch was changed to a $watchCollection, for performance reasons. This change meant that _"changing a property deeper inside the object/collection than the first level will not trigger a re-rendering"_. (A note had been added to the docs.)

3. The finally, this was changed to a $watch in b5a9053ba.

#

cc @petebacondarwin, who should know more (as he has made these changes).

[petebacondarwin]

@p0lar-bear - sorry you have had problems here. The interaction between ngModel and deep objects has always been a bit tricky: a combination of difficulty identifying what changes have occurred and keeping performance of the application reasonable.

Deep watch is the heavy hammer that is sometimes applied to fix these kinds of things but in general it kills application performance, especially if you inadvertently watch a very large object.

When we use track by with ngOptions we are basically saying that we do not care about the identity of the option object, only whatever we are tracking (e.g. id). In that case it is consistent to say that the selected item is not necessarily going to be the same object as the one in the model that we passed in.

So as you suggest in your original issue post, the workaround is to use the id that you are tracking to find the original model object if you want to make changes. Here is a link to a Plunker, which does indeed do this, https://plnkr.co/edit/wBS6IwQoV55XjM4xN7Vx?p=preview.

I would be willing to merge a PR that updates the documentation with this use case if someone puts one together.

[gkalpak]

TBH, I find this very unintuitive, but I understand there might be no ideal solution. Accepting that this ship has sailed, I am still wondering why b5a9053ba changed $watchCollection to $watch for non-multiple selects. @petebacondarwin, any ideas?

[petebacondarwin]

What that commit is referring to is watching the current "track by" value. In the case of single select then we only have one "track by" value to watch. If the select is multiple then we must watch a collection of them.

[gkalpak]

In the case of single select then we only have one "track by" value to watch

Yeah, but afaict the previous commits were using $watchCollection on that single value as an optimization over deep wathing it, in order to be able to detect (shallow) changes. b5a9053ba switches to $watch without an explanation why and I am not usre it was intentional (looks more like an oversight).

I mean 47f9fc3e7 explicitly says we are supposed to detect (shallow) changes in the selected value, but then b5a9053ba changes that (by switching to $watch) without mentioning anything.

[petebacondarwin]

I don't think that is what happened in b5a9053. See

[p0lar-bear]

@petebacondarwin Your linked plunker is identical to the one I provided, but I understand what you mean (as I've implemented said workaround in my own code, anyway).

I hate to be that guy that just gripes about stuff without having a solution at the ready, but at the moment I don't have time to spare to figure out how to properly fork this repo, make changes, and submit a PR. Nor am I certain on how to phrase what to put in the docs...

Nonetheless, thank you for listening and responding so promptly to this issue, and clarifying why track by behaves the way it does. And apologies if I came off as brash or heavy-handed about how I thought it should work.

[petebacondarwin]

Sorry about the plunker: https://plnkr.co/edit/tQIGqdcM373pzSZfDasd?p=preview

[petebacondarwin]

I thought your tone was spot on @p0lar-bear :-)

[gkalpak]

@petebacondarwin, the diff you posted shows old (for both multiple and non-multiple) vs new for multiple.
The new implementation for non-multiple is a few lines above (and is using $watch).

[petebacondarwin]

Oh you mean this:

     // We also need to watch to see if the internals of the model changes, since       
      // ngModel only watches for object identity change        
      if (ngOptions.trackBy) {      
        scope.$watchCollection(attr.ngModel, function() { ngModelCtrl.$render(); });        
      }

We were are actually having to watch the model because at that time we didn't have a way of watching the trackBy value directly. After that commit we watched the result of generating the trackBy value.

So we were inadvertently watching the model "as a collection" but this was never the aim. The trackBy value was always supposed to be value thing and not a complex object.

[gkalpak]

So we were inadvertently watching the model "as a collection" but this was never the aim.

Well...reading through 47f9fc3e7, I get a feeling that this was exactly the aim :grin:

- * <div class="alert alert-warning">
- * **Note:** By default, `ngModel` compares by reference, not value. This is important when binding to an
- * array of objects. See an example [in this jsfiddle](http://jsfiddle.net/qWzTb/). When using `track by`
- * in an `ngOptions` expression, however, deep equality checks will be performed.
- * </div>
+ * ## Complex Models (objects or collections)
+ *
+ * **Note:** By default, `ngModel` watches the model by reference, not value. This is important when
+ * binding any input directive to a model that is an object or a collection.
+ *
+ * Since this is a common situation for `ngOptions` the directive additionally watches the model using
+ * `$watchCollection` when the select has the `multiple` attribute or when there is a `track by` clause in
+ * the options expression. This allows ngOptions to trigger a re-rendering of the options even if the actual
+ * object/collection has not changed identity but only a property on the object or an item in the collection
+ * changes.
+ *
+ * Note that `$watchCollection` does a shallow comparison of the properties of the object (or the items in the collection
+ * if the model is an array). This means that changing a property deeper inside the object/collection that the
+ * first level will not trigger a re-rendering.

Note especially this part (emphasis mine):

Since this is a common situation for ngOptions the directive additionally watches the model using
$watchCollection when the select has the multiple attribute or when there is a track by clause in
the options expression.

[petebacondarwin]

Right, but the point was that the implementation required that we watch the model as a collection because we were not watching the trackBy value. When using trackBy (on a non-multiple select) we are saying; we don't care what the actual object identity is (or what its properties are) we only care about the trackBy value, right?

Previous to this commit, we were watching the object deeply, which was, again, only to capture a trigger for the trackBy value. One could make the same argument, for that change.

On a multiple select (with trackBy) we must watch a collection of the trackBy values since we could be selecting more than one.

Am I missing something here?

I guess it would do little harm to add that watch back in just for !multiple && trackBy.

[gkalpak]

but the point was that the implementation required that we watch the model as a collection because we were not watching the trackBy value.

I see. I thought the point of watching was that (since we started to copy() tracked-by values instead of using the original object) we needed to keep the two copies in sync (in case any property changed).

But now I realize (correct me if I'm wrong), the point was to compute the track-by value. So essentially, we are saying that (in case of track by), the ngModel will be a copy of the selected object and not the object itself (in which case I agree it makes little sense to try and keep the two copies in sync).

So, if that is indeed the case, I think we should just add a note in the docs and leave the code as is. That would mean that OP's usecase (_"I expected changes to the ngModel value to propagate back to the source array/object"_) is not supported, but I think that is OK, since even if we copied changes across, other usecases (that relied on objects being the same instance) would fail anyway. I prefer being explicit about what is and isn't supported, than trying to semi-support something (and hurt performance in the process too).

#

@p0lar-bear, you might want to look into using select + ngRepeat, which doesn't suffer from the same issue:

<select ng-model="currentItem">
  <option ng-value="item" ng-repeat="item in items track by item.id">
    {{ item.name }}
  </option>
</select>

[ericmdantas]

Here are two more examples showing the unexpected behavior (copied from #16437):

---

In the first example, the items[1] information changes, but ng-model is not updated. When the user selects another option and selects the previous option again, ng-model is updated correctly.

Manipulating the list by reference

Live demo: http://jsbin.com/kexobeyahe/1/edit?html,js,output

<!DOCTYPE html>
<html ng-app="app">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
  <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.8/angular.min.js"></script>
</head>
<body ng-controller="Controller as c">
  <main>
    <select ng-model="c.wrapper.item"
            ng-options="it as it.name for it in c.items track by it.id"></select>

    <br>
    <br>

    <label>Model</label>
    <pre>{{c.wrapper | json}}</pre>

    <br>
    <br>

    <label>List</label>
    <pre>{{c.items | json}}</pre>
  </main>
</body>
</html>

console.clear()

angular
  .module('app', [])
  .controller('Controller', function($scope, $timeout) {
    this.items = [
      {
        id: 1,
        code: 111,
        name: 'a'
      },
      {
        id: 2,
        code: 222,
        name: 'b'
      },
      {
        id: 3,
        code: 333,
        name: 'c'
      }
    ]

    this.wrapper = {
      // points to items[1], because of the id
      item: {
        id: 2, 
        code: 333, 
        name: 'b'
      }
    }

    $timeout(() => {
      // wrapper.item doesn't change, but it should
      this.items[1].code = 9999999
    }, 1000)
  })

---

In the second example, the items[1] information changes, but ng-model is not updated. But, when the user selects another option and selects the previous option again, ng-model is still not updated correctly - it's stuck with the first value

Manipulating the list by copy

Live demo: http://jsbin.com/jitiwemofa/edit?html,js,output

<!DOCTYPE html>
<html ng-app="app">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
  <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.8/angular.min.js"></script>
</head>
<body ng-controller="Controller as c">
  <main>
    <select ng-model="c.wrapper.item"
            ng-options="it as it.name for it in c.items track by it.id"></select>

    <br>
    <br>

    <label>Model</label>
    <pre>{{c.wrapper | json}}</pre>

    <br>
    <br>

    <label>List</label>
    <pre>{{c.items | json}}</pre>
  </main>
</body>
</html>

console.clear()

angular
  .module('app', [])
  .controller('Controller', function($scope, $timeout) {
    var list = [
      {
        id: 1,
        code: 111,
        name: 'a'
      },
      {
        id: 2,
        code: 222,
        name: 'b'
      },
      {
        id: 3,
        code: 333,
        name: 'c'
      }
    ]

    this.items = angular.copy(list)

    this.wrapper = {
      // points to items[1], because of the id
      item: {
        id: 2,
        code: 222,
        name: 'b'
      }
    }

    $timeout(() => {
      var newList = angular.copy(list)

      // newList[1] changes correctly
      // wrapper.item doesn't change, but it should
      newList[1].code = 9999999

      // changes to the list passed to the list the view knows about
      this.items = newList
    }, 1000)
  })

[AnderssonPeter]

Had this issue today, I was so confused why === yielded false.