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

fix(select): Add exception for unsupported ngOptions use case #9290

Closed
wants to merge 1 commit into from
Closed

fix(select): Add exception for unsupported ngOptions use case #9290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/ng/directive/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
// - optionGroupsCache[?][0] is the parent: either the SELECT or OPTGROUP element
optionGroupsCache = [[{element: selectElement, label:''}]];

if (track && match[2] && valueName !== match[1]) {
throw ngOptionsMinErr('trackSelect',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no minerr file being added in this PR, that I can see. Do we already have a trackSelect error used for something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like we do, please add an error file to https://github.com/angular/angular.js/tree/master/docs/content/error/ngOptions

"Do not use 'track by' when your select ('{0}') is different from your value ('{1}')",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an english speaker, I have no idea what this error message is telling me. "Do not use 'track by' when your select (what is "my select"?) is different from your value (??? what!?)"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are the terms used in the documentation to describe the different parts of the ng-options expression (see https://docs.angularjs.org/api/ng/directive/select).
I'm not too happy about the wording either. The insertion of the actual parsed subexpression should make clear what is meant though. Would you be ok with putting select and value in quotes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather we didn't have this limitation in the first place and just made this work the way people expect it to work.

match[1], valueName);
}

if (nullOption) {
// compile the element since there might be bindings in it
$compile(nullOption)(scope);
Expand Down
9 changes: 9 additions & 0 deletions test/ng/directive/selectSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,15 @@ describe('select', function() {
expect(element.val()).toEqual('4');
});

it('should throw an error when trying to combine track by with a complex select expression', function() {
expect(function() {
createSelect({
'ng-model': 'selected',
'ng-options': 'item.id as item.name for item in values track by item.id'
});
}).toThrowMinErr('ngOptions','trackSelect', "Do not use 'track by' when your select ('item.id') is different from your value ('item')");
});


it('should bind to scope value through experession', function() {
createSelect({
Expand Down