fix(select): update labels

[btford]

Closes #9025

[btford]

I think that I can possibly implement this more efficiently but I wanted a proof of concept diff that wasn't totally insane.

[caitp]

I'm sure I'm just not seeing it, but where is valueName defined?

[caitp]

nevermind, apparently was searching for nameValue --- so what are we doing, registering a $watchCollection in the middle of setup, for a single option?

[petebacondarwin]

What this is doing is saying, every digest loop, build a new array of all the display names, which are generated from expressions, which usually rely upon the valueName. Then if this array changes then re-render.

It is rather heavy weight - if we could say that these expressions only ever depended upon the values in the ng-option expression then it could be much simpler.

[caitp]

Isn't the goal here to update labels dynamically though? in which case the values wouldn't matter much.

I think we could do a better job by doing the work in a single watcher, but it would be hard to read/reason about (the select directive is already pretty hard to read and reason about)

[petebacondarwin]

The trouble with this is that we then also have to do the same for things like groupByFn and so on.

Really what I'd like to do is believe that all these functions are only affected by the valuesFn and so we could just do

        scope.$watch(valuesFn, scheduleRendering, true);

instead of the

        scope.$watchCollection(valuesFn, scheduleRendering);

I tested this and it passes your test. It would of course fail if the displayFn et al also relied on some input outside of the values array.

[caitp]

we don't have a benchmark for <select> yet, afaik (it is possible that I'm wrong) --- the second watchCollection may be quicker than using object equality

[petebacondarwin]

The second watchCollection is still in there with @btford's fix. What my suggestion would get rid of is @btford's new watchCollection

[btford]

We're still better off than re-rendering on each digest, no?

Is this worth implementing a benchmark for?

[jeffbcross]

@btford was this closed by 4627410?