ngOptions "track by" makes copies of array elements, expected references

[p0lar-bear]

I'm submitting a ...

• 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 6a03ca2 (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 47f9fc3, 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 b5a9053.

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 b5a9053 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. b5a9053 switches to $watch without an explanation why and I am not usre it was intentional (looks more like an oversight).

I mean 47f9fc3 explicitly says we are supposed to detect (shallow) changes in the selected value, but then b5a9053 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