Skip to content

Commit 526498a

Browse files
jeffbcrossjeffbcross
committed
fix(select): add basic track by and select as support
Instead of throwing an error when using "track by" and "select as" expressions, ngOptions will assume that the track by expression is valid, and will use it to compare values. Closes angular#6564
1 parent b90f5e5 commit 526498a

File tree

3 files changed

+170
-21
lines changed

3 files changed

+170
-21
lines changed

‎docs/content/error/ngOptions/trkslct.ngdoc

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
@fullName Comprehension expression cannot contain both `select as` and `track by` expressions.
44
@description
55

6+
NOTE: This error was introduced in 1.3.0-rc.5, and was removed for 1.3.0-rc.6 in order to
7+
accommodate existing apps that write complex enough `track by` expressions to work with `select as`.
8+
69
This error occurs when 'ngOptions' is passed a comprehension expression that contains both a
710
`select as` expression and a `track by` expression. These two expressions are fundamentally
811
incompatible.

‎src/ng/directive/select.js

+6-12
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
363363
//re-usable object to represent option's locals
364364
locals = {};
365365

366-
if (trackFn && selectAsFn) {
367-
throw ngOptionsMinErr('trkslct',
368-
"Comprehension expression cannot contain both selectAs '{0}' " +
369-
"and trackBy '{1}' expressions.",
370-
selectAs, track);
371-
}
372-
373366
if (nullOption) {
374367
// compile the element since there might be bindings in it
375368
$compile(nullOption)(scope);
@@ -460,7 +453,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
460453
function createIsSelectedFn(viewValue) {
461454
var selectedSet;
462455
if (multiple) {
463-
if (!selectAs && trackFn && isArray(viewValue)) {
456+
if (trackFn && isArray(viewValue)) {
464457

465458
selectedSet = new HashMap([]);
466459
for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) {
@@ -470,15 +463,16 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
470463
} else {
471464
selectedSet = new HashMap(viewValue);
472465
}
473-
} else if (!selectAsFn && trackFn) {
466+
} else if (trackFn) {
474467
viewValue = callExpression(trackFn, null, viewValue);
475468
}
469+
476470
return function isSelected(key, value) {
477471
var compareValueFn;
478-
if (selectAsFn) {
479-
compareValueFn = selectAsFn;
480-
} else if (trackFn) {
472+
if (trackFn) {
481473
compareValueFn = trackFn;
474+
} else if (selectAsFn) {
475+
compareValueFn = selectAsFn;
482476
} else {
483477
compareValueFn = valueFn;
484478
}

‎test/ng/directive/selectSpec.js

+161-9
Original file line numberDiff line numberDiff line change
@@ -817,22 +817,174 @@ describe('select', function() {
817817
});
818818

819819

820+
/**
821+
* This behavior is broken and should probably be cleaned up later as track by and select as
822+
* aren't compatible.
823+
*/
820824
describe('selectAs+trackBy expression', function() {
821825
beforeEach(function() {
822-
scope.arr = [{id: 10, label: 'ten'}, {id:'20', label: 'twenty'}];
823-
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
826+
scope.arr = [{subItem: {label: 'ten', id: 10}}, {subItem: {label: 'twenty', id: 20}}];
827+
scope.obj = {'10': {subItem: {id: 10, label: 'ten'}}, '20': {subItem: {id: 20, label: 'twenty'}}};
824828
});
825829

826830

827-
it('should throw a helpful minerr', function() {
828-
expect(function() {
831+
it('It should use the "value" variable to represent items in the array as well as for the ' +
832+
'selected values in track by expression (single&array)', function() {
833+
createSelect({
834+
'ng-model': 'selected',
835+
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
836+
});
837+
838+
// First test model -> view
839+
840+
scope.$apply(function() {
841+
scope.selected = scope.arr[0].subItem;
842+
});
843+
expect(element.val()).toEqual('0');
844+
845+
scope.$apply(function() {
846+
scope.selected = scope.arr[1].subItem;
847+
});
848+
expect(element.val()).toEqual('1');
849+
850+
// Now test view -> model
851+
852+
element.val('0');
853+
browserTrigger(element, 'change');
854+
expect(scope.selected).toBe(scope.arr[0].subItem);
855+
856+
// Now reload the array
857+
scope.$apply(function() {
858+
scope.arr = [{
859+
subItem: {label: 'new ten', id: 10}
860+
},{
861+
subItem: {label: 'new twenty', id: 20}
862+
}];
863+
});
864+
expect(element.val()).toBe('0');
865+
});
866+
867+
868+
it('It should use the "value" variable to represent items in the array as well as for the ' +
869+
'selected values in track by expression (multiple&array)', function() {
870+
createSelect({
871+
'ng-model': 'selected',
872+
'multiple': true,
873+
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
874+
});
875+
876+
// First test model -> view
877+
878+
scope.$apply(function() {
879+
scope.selected = [scope.arr[0].subItem];
880+
});
881+
expect(element.val()).toEqual(['0']);
882+
883+
scope.$apply(function() {
884+
scope.selected = [scope.arr[1].subItem];
885+
});
886+
expect(element.val()).toEqual(['1']);
887+
888+
// Now test view -> model
889+
890+
element.find('option')[0].selected = true;
891+
element.find('option')[1].selected = false;
892+
browserTrigger(element, 'change');
893+
expect(scope.selected).toEqual([scope.arr[0].subItem]);
894+
895+
// Now reload the array
896+
scope.$apply(function() {
897+
scope.arr = [{
898+
subItem: {label: 'new ten', id: 10}
899+
},{
900+
subItem: {label: 'new twenty', id: 20}
901+
}];
902+
});
903+
expect(element.val()).toEqual(['0']);
904+
});
829905

830-
createSelect({
831-
'ng-model': 'selected',
832-
'ng-options': 'item.id as item.name for item in values track by item.id'
833-
});
834906

835-
}).toThrowMinErr('ngOptions', 'trkslct', "Comprehension expression cannot contain both selectAs 'item.id' and trackBy 'item.id' expressions.");
907+
it('It should use the "value" variable to represent items in the array as well as for the ' +
908+
'selected values in track by expression (multiple&object)', function() {
909+
createSelect({
910+
'ng-model': 'selected',
911+
'multiple': true,
912+
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
913+
});
914+
915+
// First test model -> view
916+
917+
scope.$apply(function() {
918+
scope.selected = [scope.obj['10'].subItem];
919+
});
920+
expect(element.val()).toEqual(['10']);
921+
922+
923+
scope.$apply(function() {
924+
scope.selected = [scope.obj['10'].subItem];
925+
});
926+
expect(element.val()).toEqual(['10']);
927+
928+
// Now test view -> model
929+
930+
element.find('option')[0].selected = true;
931+
element.find('option')[1].selected = false;
932+
browserTrigger(element, 'change');
933+
expect(scope.selected).toEqual([scope.obj['10'].subItem]);
934+
935+
// Now reload the object
936+
scope.$apply(function() {
937+
scope.obj = {
938+
'10': {
939+
subItem: {label: 'new ten', id: 10}
940+
},
941+
'20': {
942+
subItem: {label: 'new twenty', id: 20}
943+
}
944+
};
945+
});
946+
expect(element.val()).toEqual(['10']);
947+
});
948+
949+
950+
it('It should use the "value" variable to represent items in the array as well as for the ' +
951+
'selected values in track by expression (single&object)', function() {
952+
createSelect({
953+
'ng-model': 'selected',
954+
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
955+
});
956+
957+
// First test model -> view
958+
959+
scope.$apply(function() {
960+
scope.selected = scope.obj['10'].subItem;
961+
});
962+
expect(element.val()).toEqual('10');
963+
964+
965+
scope.$apply(function() {
966+
scope.selected = scope.obj['10'].subItem;
967+
});
968+
expect(element.val()).toEqual('10');
969+
970+
// Now test view -> model
971+
972+
element.find('option')[0].selected = true;
973+
browserTrigger(element, 'change');
974+
expect(scope.selected).toEqual(scope.obj['10'].subItem);
975+
976+
// Now reload the object
977+
scope.$apply(function() {
978+
scope.obj = {
979+
'10': {
980+
subItem: {label: 'new ten', id: 10}
981+
},
982+
'20': {
983+
subItem: {label: 'new twenty', id: 20}
984+
}
985+
};
986+
});
987+
expect(element.val()).toEqual('10');
836988
});
837989
});
838990

0 commit comments

Comments
 (0)