Regression using ngOptions asynchronously in angular 1.3 #9714

FrenchDilettante · 2014-10-21T10:08:21Z

Hi guys,

Just wanted to start by saying that Angular is a wonderful project and I'm having a great time working with it :)

I have an example of the issue in this jsFiddle: http://jsfiddle.net/88aw4yyq/

Consider this HTML:

<select name="test"
        ng-model="test"
        ng-options="opt.id as opt.label for opt in availableOptions">
</select>

And this controller:

$scope.test = '';
$scope.availableOptions = [];

$timeout(function () {
    $scope.availableOptions = [
        {id: '', label: 'Please select...'},
        {id: 1, label: 'opt1'},
        {id: 2, label: 'opt2'}
    ];
}, 1000);

The $timeout simulates any regular AJAX request. {id: '', label: 'Please select...'} is added for convenience for the user.

Expected behavior: 'Please select...' should be selected once the data is loaded.

Actual behavior: An empty option is added on top in the select field.

Angular versions: 1.3.0 this is a regression, 1.2.x was working fine.

Browser versions: Tested on Fx/Chrome (latest versions) on both OS X & Windows.

If anyone else is encountering the issue, here is a workaround:

$scope.test = '';
$scope.availableOptions = [
    {id: '', label: 'Please select...'}
];

$timeout(function () {
    $scope.availableOptions = $scope.availableOptions.concat([
        {id: 1, label: 'opt1'},
        {id: 2, label: 'opt2'}
    ]);
}, 1000);

The text was updated successfully, but these errors were encountered:

anotherchrisberry · 2014-10-29T05:19:17Z

I just ran into this same issue (plunker here: http://plnkr.co/edit/NgPGrklA3ng6bmnPjxmw?p=preview) and ended up with the same solution (pre-filling the options holder with the known selected value). The workaround is kind of ugly but not terrible.

Tested in Firefox, Chrome and Safari but did not see the behavior in Firefox. Only tested on OS X.

ghost · 2014-10-29T11:52:46Z

This is a big problem for our group. All versions of AngularJS are okay up until the last 1.3.0 version. There's no simple workaround for us as every select box in our application is populated the same way and now none of them work as expected. I hope this bug gets fixed ASAP. Seems like it was introduced with 1.3.0. Now going back to using the RC release while waiting for someone to at least back out the change in the options that was made just prior to the 1.3.0 release :-(

ajohnston256 · 2014-11-22T22:59:43Z

Can you assign this to me? I think I see the problem, and I'll try to get a pull request ready for tomorrow.

Fix regression in angular 1.3 where empty string is added to select Closes angular#9714

Fix angular 1.3 regression where empty string was shown Closes angular#9714

Otherwise, if the removed option was the empty option (value ''), and the currently selected option had a value of 0, the select would think that the currently selected option had been removed, causing the unknown option to be added again. Fixes angular#9714 Fixes angular#10115 Closes angular#10203

pisaacs · 2014-11-27T16:02:24Z

Sounds like this is the root cause for the following issue?
http://stackoverflow.com/questions/26374226/angularjs-1-3-0-no-longer-shows-default-selection-when-ng-options-references-dif

Otherwise, if the removed option was the empty option (value ''), and the currently selected option had a value of 0, the select would think that the currently selected option had been removed, causing the unknown option to be added again. Fixes angular#9714 Fixes angular#10115 Closes angular#10203

JohnYoungers · 2014-12-02T15:59:37Z

Is this separate from the issue outlined in this plunker? The value isn't displaying in any 1.3.x

http://plnkr.co/edit/7nBZLR8siWn1cfck9LIH?p=preview

csbenjamin · 2014-12-02T16:19:49Z

have a look at this one

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

We have the @jayoungers related issue more clean and another issue where ng-options selects the wrong option.

anotherchrisberry · 2014-12-02T16:37:09Z

Should we see the values selected in (http://plnkr.co/edit/P9WCLicfJjLVCr8nBdqn?p=preview)?

If this issue is fixed, I would expect to see this:

But instead I see this:

gkalpak · 2014-12-02T17:00:19Z

Interestingly enough, adding a track by expression fixed the issue:

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

JohnYoungers · 2014-12-02T17:27:14Z

Something definitely wonky's going on; you would think both of these selects should act the same way (first instantly has the items, 2nd is delayed):

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

adding/removing the track by piece results in the other working

Narretz · 2014-12-02T18:17:20Z

Looks like I only fixed a specifc case, reopening

petebacondarwin · 2014-12-14T22:38:34Z

Actually looking again I think the problem is that these lines: https://github.com/angular/angular.js/blob/v1.3.6/src/ng/directive/select.js#L694-L700

          forEach(labelMap, function(count, label) {
            if (count > 0) {
              selectCtrl.addOption(label);
            } else if (count < 0) {
              selectCtrl.removeOption(label);
            }
          });

Are using the ctrl.addOption and ctrl.removeOption API incorrectly. If you look they are passing in the label of the option, whereas the functions expect to receive the value of the option. @lgalfaso and @shahata - what do you think?

Narretz · 2014-12-15T13:01:13Z

@petebacondarwin We should investigate that, thanks for checking this out.

edit: I think that's correct: if an option has the label '1', addOption('1') will set the select element's val to '1', which means the second option is selected.

What's strange is that the follwing reproductions work fine in FF34, but fail in Chrome:
http://plnkr.co/edit/7nBZLR8siWn1cfck9LIH?p=preview
http://jsfiddle.net/ayvLah0v/1/ (no track-by examples)

Maybe just a timing issue?

petebacondarwin · 2014-12-15T13:25:07Z

Yes - weird. I still think that the removeOption method is to blame...

joelross · 2014-12-16T11:42:51Z

Just copy/pasting my initial comment from #9691 :

Suspected cause: this function expects a value but this call sends it a label, causing this line to unselect all options (as this is not a known value). In my case, the received argument value is indeed the string label of the option, where the actual value of the option (according to the jQuery component) seems to be a numerical index.

Just for the record, I solved my problem by commenting this line (but I can imagine this causes other issues I am not aware of).

I have been using this small fix without undesirable side-effects in my project since then.

(JSFiddle) if you set a breakpoint on the $element.val(value); line, you will see that value is "obj1" and $element is

<select ng-model="selected" ng-options="i as i for i in items" class="ng-pristine ng-untouched ng-valid">
    <option value="0" selected="selected" label="obj1">obj1</option>
    <option value="1" label="obj2">obj2</option>
</select>

$element.val(value) then seems to be simply discarded on some browsers, but causes $element.val() to become null or "" on Chromium.

petebacondarwin · 2014-12-17T22:27:35Z

@Narretz and I paired on this tonight. It turns out that there is a significant problem inside the select directive to do with synchronizing the option elements with the option items (objects, strings, etc) in the ngOptions collection. Currently there are two strategies going on inside the directive's $render and the SelectController to identify (or "track") these.

use the label of the option as the identifier
use the trackByFn/index/key of the option as the identifier in the case where the ngOptions collection is using track by or is just an array or an object respectively.

Neither of these strategies is adequate. It is feasible to have multiple options with the same label and in any case the underlying selected option could change if the option's label changes compared to its "value". In the case of arrays, using index is flawed as the index of an option can change if it is moved around in the array (or even if another item is simply removed from the front of the array).

The suggested fix is to follow the same strategy as ngRepeat which is to use Angular HashMaps to track these items, where the value of the thing being stored is what would be computed when an option were being written to the viewValue. This way we are passing around real objects (with hidden but pure surrogate key identifiers - in the form of the $$hashKey property) and not relying on potentially mutable natural keys.

This will involve quite a bit of internal refactoring but should hopefully not impact on the public API at all.

JohnYoungers · 2014-12-17T22:40:57Z

Sounds great guys!

If anyone runs into a similar problem in the meantime, the temporary solution I went with was adding a ng-if to one of the select's parent elements to ensure the select doesn't get created until my collections are retrieved from the server.

angrytoro · 2014-12-29T02:03:38Z

maybe it cause by the code

self.addOption = function(value, element) {
    assertNotHasOwnProperty(value, '"option value"');
    optionsMap[value] = true;

    if (ngModelCtrl.$viewValue == value) {
      $element.val(value);
      if (unknownOption.parent()) unknownOption.remove();
    }
    // Workaround for https://code.google.com/p/chromium/issues/detail?id=381459
    // Adding an <option selected="selected"> element to a <select required="required"> should
    // automatically select the new element
    if (element && element[0].hasAttribute('selected')) {
      element[0].selected = true;
    }
};

and the code make the select don't show the real value.

if (ngModelCtrl.$viewValue == value) {
      $element.val(value);
      if (unknownOption.parent()) unknownOption.remove();
}

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: Although it is internal, and not documented, this commit changes the actual strings that are stored as the value of each `<option>` element. We now store a string that is computed by calling `hashKey` on the item in the options collection; before it was the index or key value of the item in the collection. Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. Closes angular#9714

petebacondarwin · 2015-01-04T14:33:11Z

@angrytoro - this is the right idea but ngOptions needs a full refactoring, I have provided a PR in #10639...

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: When using `ngOptions`: the directive applies a surrogate key as the value of the `<option>` element. This commit changes the actual string used as the surrogate key. We now store a string that is computed by calling `hashKey` on the item in the options collection; previously it was the index or key of the item in the collection. Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. You may find that you can use the `track by` feaure of `ngOptions` as this provides the ability to specify the key that is stored. Closes angular#9714

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: When using `ngOptions`: the directive applies a surrogate key as the value of the `<option>` element. This commit changes the actual string used as the surrogate key. We now store a string that is computed by calling `hashKey` on the item in the options collection; previously it was the index or key of the item in the collection. (This is in keeping with the way that the unknown option value is represented in the select directive.) Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. You may find that you can use the `track by` feaure of `ngOptions` as this provides the ability to specify the key that is stored. Closes angular#9714

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: When using `ngOptions`: the directive applies a surrogate key as the value of the `<option>` element. This commit changes the actual string used as the surrogate key. We now store a string that is computed by calling `hashKey` on the item in the options collection; previously it was the index or key of the item in the collection. (This is in keeping with the way that the unknown option value is represented in the select directive.) Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. You may find that you can use the `track by` feaure of `ngOptions` as this provides the ability to specify the key that is stored. Closes angular#9714 Closes angular#10639

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: When using `ngOptions`: the directive applies a surrogate key as the value of the `<option>` element. This commit changes the actual string used as the surrogate key. We now store a string that is computed by calling `hashKey` on the item in the options collection; previously it was the index or key of the item in the collection. (This is in keeping with the way that the unknown option value is represented in the select directive.) Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. You may find that you can use the `track by` feaure of `ngOptions` as this provides the ability to specify the key that is stored. Closes angular#8019 Closes angular#9714 Closes angular#10639

…ns are loaded async **Major reworking of select and ngOptions**: * The `SelectController` is now used as an abstraction for the `select` and `ngOptions` directives to override to get their desired behaviour * The `select` directive is completely oblivious to the ngOptions directive now - the `ngOptions` directive could be deleted without having to make any changes to the `select` directive. * Select related directives (single/multiple/ngOptions) can provide specific versions of `SelectController.writeValue` and `SelectController.readValue`, which are responsible for getting the `$viewValue` in or out of the actual `<select>` element and its `<option>` children. BREAKING CHANGE: When using `ngOptions`: the directive applies a surrogate key as the value of the `<option>` element. This commit changes the actual string used as the surrogate key. We now store a string that is computed by calling `hashKey` on the item in the options collection; previously it was the index or key of the item in the collection. (This is in keeping with the way that the unknown option value is represented in the select directive.) Before you might have seen: ``` <select ng-model="x" ng-option="i in items"> <option value="1">a</option> <option value="2">b</option> <option value="3">c</option> <option value="4">d</option> </select> ``` Now it will be something like: ``` <select ng-model="x" ng-option="i in items"> <option value="string:a">a</option> <option value="string:b">b</option> <option value="string:c">c</option> <option value="string:d">d</option> </select> ``` If your application code relied on this value, which it shouldn't, then you will need to modify your application to accommodate this. You may find that you can use the `track by` feaure of `ngOptions` as this provides the ability to specify the key that is stored. BREAKING CHANGE: When iterating over an object's properties using the `(key, value) in obj` syntax the order of the elements used to be sorted alphabetically. This was an artificial attempt to create a deterministic ordering since browsers don't guarantee the order. But in practice this is not what people want and so this change iterates over properties in the order they are returned by Object.keys(obj), which is almost always the order in which the properties were defined. Closes angular#8019 Closes angular#9714 Closes angular#10639

ghost mentioned this issue Oct 29, 2014

ngSelect appears unselected when filled after selected value is set #9691

Closed

pkozlowski-opensource added component: forms severity: broken expected use severity: regression type: bug labels Oct 30, 2014

pkozlowski-opensource added this to the 1.3.x milestone Oct 30, 2014

ajohnston256 added a commit to ajohnston256/angular.js that referenced this issue Nov 23, 2014

fix(select): allow $timeout to initialize ngOptions

c03d6b0

Fix regression in angular 1.3 where empty string is added to select Closes angular#9714

ajohnston256 added a commit to ajohnston256/angular.js that referenced this issue Nov 23, 2014

fix(select): allow ngOptions to be set in $timeout

4459d38

Fix angular 1.3 regression where empty string was shown Closes angular#9714

ajohnston256 added a commit to ajohnston256/angular.js that referenced this issue Nov 23, 2014

fix(select): allow ngOptions to be set in $timeout

62f242c

Fix angular 1.3 regression where empty string was shown Closes angular#9714

This was referenced Nov 23, 2014

fix(select): allow ngOptions to be set in $timeout #10203

Closed

Angular Test Process for <SELECT> #10201

Closed

Narretz modified the milestones: 1.3.5, 1.3.x Nov 23, 2014

Narretz self-assigned this Nov 25, 2014

Narretz mentioned this issue Nov 25, 2014

ngOptions with model = 0 and options loaded later #10213

Closed

Narretz pushed a commit to Narretz/angular.js that referenced this issue Nov 26, 2014

fix(select): allow ngOptions to be set in $timeout

8e2aa27

Fix angular 1.3 regression where empty string was shown Closes angular#9714

gkalpak mentioned this issue Nov 28, 2014

Default value with ng-options not working with same object values #10255

Closed

Narretz closed this as completed in 9fa73cb Dec 1, 2014

petebacondarwin modified the milestones: 1.3.7, 1.3.8 Dec 15, 2014

btford modified the milestones: 1.3.8, 1.3.9 Dec 19, 2014

petebacondarwin mentioned this issue Jan 4, 2015

refact(ngOptions): major refactoring... #10639

Closed

petebacondarwin closed this as completed in 7fda214 Jan 12, 2015

pisaacs mentioned this issue May 8, 2015

Work-around for major 1.3x ng-options issue? #11835

Closed

bvaughn mentioned this issue Jul 25, 2015

Select field not pre-selecting value when label and value are equal bvaughn/angular-form-for#145

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regression using ngOptions asynchronously in angular 1.3 #9714

Regression using ngOptions asynchronously in angular 1.3 #9714

FrenchDilettante commented Oct 21, 2014

anotherchrisberry commented Oct 29, 2014

ghost commented Oct 29, 2014

ajohnston256 commented Nov 22, 2014

pisaacs commented Nov 27, 2014

JohnYoungers commented Dec 2, 2014

csbenjamin commented Dec 2, 2014

anotherchrisberry commented Dec 2, 2014

gkalpak commented Dec 2, 2014

JohnYoungers commented Dec 2, 2014

Narretz commented Dec 2, 2014

petebacondarwin commented Dec 14, 2014

Narretz commented Dec 15, 2014

petebacondarwin commented Dec 15, 2014

joelross commented Dec 16, 2014

petebacondarwin commented Dec 17, 2014

JohnYoungers commented Dec 17, 2014

angrytoro commented Dec 29, 2014

petebacondarwin commented Jan 4, 2015

Regression using ngOptions asynchronously in angular 1.3 #9714

Regression using ngOptions asynchronously in angular 1.3 #9714

Comments

FrenchDilettante commented Oct 21, 2014

anotherchrisberry commented Oct 29, 2014

ghost commented Oct 29, 2014

ajohnston256 commented Nov 22, 2014

pisaacs commented Nov 27, 2014

JohnYoungers commented Dec 2, 2014

csbenjamin commented Dec 2, 2014

anotherchrisberry commented Dec 2, 2014

gkalpak commented Dec 2, 2014

JohnYoungers commented Dec 2, 2014

Narretz commented Dec 2, 2014

petebacondarwin commented Dec 14, 2014

Narretz commented Dec 15, 2014

petebacondarwin commented Dec 15, 2014

joelross commented Dec 16, 2014

petebacondarwin commented Dec 17, 2014

JohnYoungers commented Dec 17, 2014

angrytoro commented Dec 29, 2014

petebacondarwin commented Jan 4, 2015