Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit abdaab7

Browse files
committed
fix(select): throw for selectAs and trackBy
trackBy and selectAs have never worked together, and are fundamentally incompatible since model changes cannot deterministically be reflected back to the view. This change throws an error to help developers better understand this scenario.
1 parent aad6095 commit abdaab7

File tree

3 files changed

+55
-170
lines changed

3 files changed

+55
-170
lines changed
+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
@ngdoc error
2+
@name ngOptions:trkslct
3+
@fullName Comprehension expression cannot contain both `select as` and `track by` expressions.
4+
@description
5+
6+
This error occurs when 'ngOptions' is passed a comprehension expression that contains both a
7+
`select as` expression and a `track by` expression. These two expressions are fundamentally
8+
incompatible.
9+
10+
* Example of bad expression: `<select ng-options="item.subItem as item.label for item in values track by item.id" ng-model="selected">`
11+
`values: [{id: 1, label: 'aLabel', subItem: {name: 'aSubItem'}}, {id: 2, label: 'bLabel', subItem: {name: 'bSubItem'}}]`,
12+
`$scope.selected = {name: 'aSubItem'};`
13+
* track by is always applied to `value`, with purpose to preserve the selection,
14+
(to `item` in this case)
15+
* To calculate whether an item is selected, `ngOptions` does the following:
16+
1. apply `track by` to the values in the array:
17+
In the example: [1,2]
18+
2. apply `track by` to the already selected value in `ngModel`:
19+
In the example: this is not possible, as `track by` refers to `item.id`, but the selected
20+
value from `ngModel` is `{name: aSubItem}`.
21+
22+
Here's an example of how to make this example work by using `track by` without `select as`:
23+
24+
```
25+
<select ng-model="selected" ng-options="item.label for item in values track by item.id">
26+
```
27+
28+
Note: This would store the whole `item` as the model to `scope.selected` instead of `item.subItem`.
29+
30+
For more information on valid expression syntax, see 'ngOptions' in {@link ng.directive:select select} directive docs.

src/ng/directive/select.js

+15-9
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ var ngOptionsMinErr = minErr('ngOptions');
3636
* </div>
3737
*
3838
* <div class="alert alert-info">
39-
* **Note:** Using `selectAs` will bind the result of the `selectAs` expression to the model, but
40-
* the value of the `select` and `option` elements will be either the index (for array data sources)
39+
* **Note:** Using `select as` will bind the result of the `select as` expression to the model, but
40+
* the value of the `<select>` and `<option>` html elements will be either the index (for array data sources)
4141
* or property name (for object data sources) of the value within the collection.
4242
* </div>
4343
*
@@ -79,19 +79,18 @@ var ngOptionsMinErr = minErr('ngOptions');
7979
* even when the options are recreated (e.g. reloaded from the server).
8080
8181
* <div class="alert alert-info">
82-
* **Note:** Using `selectAs` together with `trackexpr` is not possible (and will throw).
83-
* TODO: Add some nice reasoning here, add a minErr and a nice error page.
84-
* reasoning:
82+
* **Note:** Using `select as` together with `trackexpr` is not possible (and will throw).
83+
* Reasoning:
8584
* - Example: <select ng-options="item.subItem as item.label for item in values track by item.id" ng-model="selected">
8685
* values: [{id: 1, label: 'aLabel', subItem: {name: 'aSubItem'}}, {id: 2, label: 'bLabel', subItem: {name: 'bSubItemß'}}],
8786
* $scope.selected = {name: 'aSubItem'};
88-
* - trackBy is always applied to `value`, with purpose to preserve the selection,
87+
* - track by is always applied to `value`, with purpose to preserve the selection,
8988
* (to `item` in this case)
9089
* - to calculate whether an item is selected we do the following:
91-
* 1. apply `trackBy` to the values in the array, e.g.
90+
* 1. apply `track by` to the values in the array, e.g.
9291
* In the example: [1,2]
93-
* 2. apply `trackBy` to the already selected value in `ngModel`:
94-
* In the example: this is not possible, as `trackBy` refers to `item.id`, but the selected
92+
* 2. apply `track by` to the already selected value in `ngModel`:
93+
* In the example: this is not possible, as `track by` refers to `item.id`, but the selected
9594
* value from `ngModel` is `{name: aSubItem}`.
9695
*
9796
* </div>
@@ -367,6 +366,13 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
367366
//re-usable object to represent option's locals
368367
locals = {};
369368

369+
if (trackFn && selectAsFn) {
370+
throw ngOptionsMinErr('trkslct',
371+
"Comprehension expression cannot contain both selectAs '{0}' " +
372+
"and trackBy '{1}' expressions.",
373+
selectAs, track);
374+
}
375+
370376
if (nullOption) {
371377
// compile the element since there might be bindings in it
372378
$compile(nullOption)(scope);

test/ng/directive/selectSpec.js

+10-161
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ describe('select', function() {
663663
});
664664

665665

666-
describe('trackBy', function() {
666+
describe('trackBy expression', function() {
667667
beforeEach(function() {
668668
scope.arr = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}];
669669
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
@@ -761,173 +761,22 @@ describe('select', function() {
761761
});
762762

763763

764-
describe('selectAs+trackBy', function() {
764+
describe('selectAs+trackBy expression', function() {
765765
beforeEach(function() {
766766
scope.arr = [{id: 10, label: 'ten'}, {id:'20', label: 'twenty'}];
767767
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
768768
});
769769

770770

771-
it('should bind selectAs expression result to scope (array&single)', function() {
772-
createSelect({
773-
'ng-model': 'selected',
774-
'ng-options': 'item.id as item.name for item in values track by item.id'
775-
});
776-
777-
scope.$apply(function() {
778-
scope.values = [{id: 10, name: 'A'}, {id: 20, name: 'B'}];
779-
scope.selected = 10;
780-
});
781-
expect(element.val()).toEqual('0');
782-
783-
scope.$apply(function() {
784-
scope.selected = 20;
785-
});
786-
expect(element.val()).toEqual('1');
787-
788-
element.val('0');
789-
browserTrigger(element, 'change');
790-
expect(scope.selected).toBe(10);
791-
});
792-
793-
794-
it('should bind selectAs expression result to scope (array&multiple)',function() {
795-
createSelect({
796-
'ng-model': 'selected',
797-
'multiple': true,
798-
'ng-options': 'item.id as item.name for item in values track by item.id'
799-
});
800-
801-
scope.$apply(function() {
802-
scope.values = [{id: 10, name: 'A'}, {id: 20, name: 'B'}];
803-
scope.selected = [10];
804-
});
805-
expect(element.val()).toEqual(['0']);
806-
807-
scope.$apply(function() {
808-
scope.selected = [20];
809-
});
810-
expect(element.val()).toEqual(['1']);
811-
812-
element.children(0).attr('selected', 'selected');
813-
element.children(1).attr('selected', 'selected');
814-
browserTrigger(element, 'change');
815-
expect(scope.selected).toEqual([10, 20]);
816-
});
817-
818-
819-
it('should bind selectAs expression result to scope (object&single)', function() {
820-
createSelect({
821-
'ng-model': 'selected',
822-
'ng-options': 'value.score as value.label for (key, value) in obj track by value.score'
823-
});
824-
825-
scope.$apply(function() {
826-
scope.selected = 10;
827-
});
828-
expect(element.val()).toEqual('10');
829-
830-
scope.$apply(function() {
831-
scope.selected = 20;
832-
});
833-
expect(element.val()).toEqual('20');
834-
835-
element.val('10');
836-
browserTrigger(element, 'change');
837-
expect(scope.selected).toBe(10);
838-
});
839-
840-
841-
it('should bind selectAs expression result to scope (object&multiple)', function() {
842-
createSelect({
843-
'ng-model': 'selected',
844-
'multiple': true,
845-
'ng-options': 'value.score as value.label for (key, value) in obj track by value.score'
846-
});
847-
848-
scope.$apply(function() {
849-
scope.selected = [10];
850-
});
851-
expect(element.val()).toEqual(['10']);
852-
853-
scope.$apply(function() {
854-
scope.selected = [20];
855-
});
856-
expect(element.val()).toEqual(['20']);
857-
858-
element.find('option')[0].selected = 'selected';
859-
browserTrigger(element, 'change');
860-
expect(scope.selected).toEqual([10, 20]);
861-
});
862-
863-
864-
it('should correctly assign model if track & select expressions differ (array&single)', function() {
865-
createSelect({
866-
'ng-model': 'selected',
867-
'ng-options': 'item.label as item.label for item in arr track by item.id'
868-
});
869-
870-
scope.$apply(function() {
871-
scope.selected = 'ten';
872-
});
873-
expect(element.val()).toBe('0');
874-
875-
element.val('1');
876-
browserTrigger(element, 'change');
877-
expect(scope.selected).toBe('twenty');
878-
});
879-
880-
881-
it('should correctly assign model if track & select expressions differ (array&multiple)', function() {
882-
createSelect({
883-
'ng-model': 'selected',
884-
'multiple': true,
885-
'ng-options': 'item.label as item.label for item in arr track by item.id'
886-
});
887-
888-
scope.$apply(function() {
889-
scope.selected = ['ten'];
890-
});
891-
expect(element.val()).toEqual(['0']);
892-
893-
element.find('option')[1].selected = 'selected';
894-
browserTrigger(element, 'change');
895-
expect(scope.selected).toEqual(['ten', 'twenty']);
896-
});
897-
771+
it('should throw a helpful minerr', function() {
772+
expect(function() {
898773

899-
it('should correctly assign model if track & select expressions differ (object&single)', function() {
900-
createSelect({
901-
'ng-model': 'selected',
902-
'ng-options': 'val.label as val.label for (key, val) in obj track by val.score'
903-
});
904-
905-
scope.$apply(function() {
906-
scope.selected = 'ten';
907-
});
908-
expect(element.val()).toBe('10');
909-
910-
element.val('20');
911-
browserTrigger(element, 'change');
912-
expect(scope.selected).toBe('twenty');
913-
});
914-
915-
916-
it('should correctly assign model if track & select expressions differ (object&multiple)', function() {
917-
createSelect({
918-
'ng-model': 'selected',
919-
'multiple': true,
920-
'ng-options': 'val.label as val.label for (key, val) in obj track by val.score'
921-
});
922-
923-
scope.$apply(function() {
924-
scope.selected = ['ten'];
925-
});
926-
expect(element.val()).toEqual(['10']);
774+
createSelect({
775+
'ng-model': 'selected',
776+
'ng-options': 'item.id as item.name for item in values track by item.id'
777+
});
927778

928-
element.find('option')[1].selected = 'selected';
929-
browserTrigger(element, 'change');
930-
expect(scope.selected).toEqual(['ten', 'twenty']);
779+
}).toThrowMinErr('ngOptions', 'trkslct', "Comprehension expression cannot contain both selectAs 'item.id' and trackBy 'item.id' expressions.");
931780
});
932781
});
933782

@@ -1264,7 +1113,7 @@ describe('select', function() {
12641113
it('should bind to scope value and track/identify objects', function() {
12651114
createSelect({
12661115
'ng-model': 'selected',
1267-
'ng-options': 'item as item.name for item in values track by item.id'
1116+
'ng-options': 'item.name for item in values track by item.id'
12681117
});
12691118

12701119
scope.$apply(function() {

0 commit comments

Comments
 (0)