track By no longer working

[weisk]

Hi guys,

On v1.3.0-rc.5, track by won't set the inner value on <options> with the desired property. Didn't happen on rc.4.

See plnkr:
http://plnkr.co/edit/sScmKu?p=preview

Reproducable: always
Browsers: independent
Operating system: independent

[jeffbcross]

Hi @weisk are you referring to the fact that the selected attribute isn't being properly updated? If I inspect the value property of the select element I see the correct behavior, but I do see that the selected attributes of options aren't being updated.

[jeffbcross]

But the behavior I mentioned is also present in rc.4, so you must be referring to something else.

[weisk]

See updated plunk:

http://plnkr.co/edit/sScmKu?p=preview

I would expect that track by would allow me to choose which property is used to hold the value of the select, but as you can see in the code, its using the $index property of the repeater, not the id property I wanted.

If you change the angular version to rc4 you will see that it works fine..

[jeffbcross]

Ahh, I see. This was undocumented and untested behavior that was refactored out as we fixed other issues. In your case, since you're not using a select as expression, you could achieve a similar result using ng-repeat, but the model assignment would be the id. Example plunker: http://plnkr.co/edit/2fgiKA?p=preview

[jeffbcross]

There's also been some discussion of making ngValue more powerful to support this use case with ngRepeat, so that you could have value and model be different.

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

Where item gets assigned to myModel and the option's value becomes item.id.

[tbosch]

I think we should fix this!

[rzajac]

+1 I relied on the track by behavior in rc4 now all my protractor tests fail :(

For example option values in code like this ng-options="country.name for country in countries track by country.code" should result in option being: <option value="US">United States</option>

[jeffbcross]

@tbosch, I love your enthusiasm :). Since we've discussed this in person in some length, would you mind commenting with how you arrived at this conclusion?

I want this feature to be supported, but to change the behavior in place would actually be a breaking change now, since we've documented how values are assigned with ng-options: https://docs.angularjs.org/api/ng/directive/select

Note: Using select as will bind the result of the select as expression to the model, but the value of the <select> and <option> html elements will be either the index (for array data sources) or property name (for object data sources) of the value within the collection.

Some options:

• Add to the ngOptions comprehension expression <-- I don't like the idea of making the comprehension even more complicated than it already is.
• Extend ngValue to provide complete symmetry between ngRepeat and ngOptions (note: the problem of assigning trackBy expression to value is already possible with ngValue, but other features of comprehension expression are not, such as assigning objects to model)
• Create a configurable provider/service specifically for select/ngOptions to specify that track by values should be assigned to element value. <-- Relatively easy to implement, but I don't like the idea of creating a provider/service for this one simple use case.
• Make it work like it used to, and break everyone who's already migrated to the new way. <-- I really don't like the idea of introducing a breaking change in the first 1.3 patch release

What are your thoughts, @tbosch?

cc: @IgorMinar

[tbosch]

Talked with @IgorMinar and @jeffbcross again about this and came to the conclusion that we should revert this to the previous behavior, i.e. that when a track by expression is specified it would end up in the value of the <option>s.

[weisk]

So glad to hear !!! Thanks guys

[rzajac]

Great to hear it will be reverted 👍

[gkalpak]

Will we ever sit down and re-implement select and ngOptions properly some time or are we sticking to the current approach: patch -> break -> revert -> repeat ? 😃

_{(I know I won't like the answer, but I have to ask anyway 😛)}

[jeffbcross]

@gkalpak in rc.5 @tbosch and I added more tests and refactored/simplified it some more, so it's much better than it used to be. This feature was just something undocumented and not explicitly tested, so we missed it.

I think there's still more that could be done to improve it, and if you want to submit a PR, @tbosch and I would be happy to review it :)

[gkalpak]

@jeffbcross : I didn't see that coming :P

I know what you did (and it was very nice indeed). I am talking more about re-designing it from scratch (because if I am not mistaken, it was initially implemented when things looked a lot different than (reagarding NgModelController etc)).
No matter how much it is improved, I think it won't ever be as good as it should (and by "good" I mean simple/efficient/extensible).

Obviously, re-designing this, is not a primary focus right now (and for good reason), but if you have anything specific in mind that could be improved, I would be glad to giveit a shot :)

[jeffbcross]

@gkalpak I hope I didn't come across as snarky! I'm really appreciative of all your help with this and other issues. I don't disagree that select/ngOptions is pretty rough. We're going to focus more on getting it right for 2.0 than on re-designing it in 1.x though.

[gkalpak]

@jeffbcross: No, you didn't. (Did I sound like you did ? 😃)

It is understandable that v2.x is the main focus right now. I wonder if there is a way for someone to get more involved into 2.x stuff yet.
("We are all playing with the new cool toy (2.x), but you can tinker with this old (and somewhat broken) one (1.x)" doesn't sound thaaaaat tempting. Just saying...)

[albertboada]

+1! 👍