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

fix(ngOptions): don't throw if options are unset inside writeValue #12968

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 11 additions & 6 deletions src/ng/directive/ngOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
var optionTemplate = document.createElement('option'),
optGroupTemplate = document.createElement('optgroup');

return {
restrict: 'A',
terminal: true,
require: ['select', 'ngModel'],
link: function(scope, selectElement, attr, ctrls) {
function ngOptionsPostLink(scope, selectElement, attr, ctrls) {

var selectCtrl = ctrls[0];
var ngModelCtrl = ctrls[1];
Expand Down Expand Up @@ -448,7 +444,6 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
unknownOption.remove();
};


// Update the controller methods for multiple selectable options
if (!multiple) {

Expand Down Expand Up @@ -726,7 +721,17 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
}

}
}

return {
restrict: 'A',
terminal: true,
require: ['select', 'ngModel'],
link: {
pre: function ngOptionsPreLink(scope, selectElement, attr, ctrls) {
ctrls[0].override();
},
post: ngOptionsPostLink
}
};
}];
20 changes: 17 additions & 3 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ var SelectController =
self.hasOption = function(value) {
return !!optionsMap.get(value);
};

// Directives that provide their own option adding mechanism call this to prevent options
// from being added in the standard way
self.overriden = false;
self.override = function() {
self.overridden = true;
self.addOption = noop;
self.removeOption = noop;
};
}];

/**
Expand Down Expand Up @@ -308,7 +317,13 @@ var selectDirective = function() {
restrict: 'E',
require: ['select', '?ngModel'],
controller: SelectController,
link: function(scope, element, attr, ctrls) {
priority: 1,
link: {
pre: selectPreLink
}
};

function selectPreLink(scope, element, attr, ctrls) {

// if ngModel is not defined, we don't need to do anything
var ngModelCtrl = ctrls[1];
Expand Down Expand Up @@ -378,7 +393,6 @@ var selectDirective = function() {

}
}
};
};


Expand Down Expand Up @@ -430,7 +444,7 @@ var optionDirective = ['$interpolate', function($interpolate) {

// Only update trigger option updates if this is an option within a `select`
// that also has `ngModel` attached
if (selectCtrl && selectCtrl.ngModelCtrl) {
if (selectCtrl && selectCtrl.ngModelCtrl && !selectCtrl.overridden) {

if (valueInterpolated) {
// The value attribute is interpolated
Expand Down
78 changes: 77 additions & 1 deletion test/ng/directive/ngOptionsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

describe('ngOptions', function() {

var scope, formElement, element, $compile;
var scope, formElement, element, $compile, linkLog;

function compile(html) {
formElement = jqLite('<form name="form">' + html + '</form>');
Expand Down Expand Up @@ -104,6 +104,53 @@ describe('ngOptions', function() {
});
});

beforeEach(module(function($compileProvider, $provide) {
linkLog = [];

$compileProvider
.directive('customSelect', function() {
return {
restrict: "E",
replace: true,
scope: {
ngModel: '=',
options: '='
},
templateUrl: 'select_template.html',
link: function(scope, $element, attributes) {
scope.selectable_options = scope.options;
}
};
})

.directive('oCompileContents', function() {
return {
link: function(scope, element) {
linkLog.push('linkCompileContents');
$compile(element.contents())(scope);
}
};
});

$provide.decorator('ngOptionsDirective', function($delegate) {

var origPreLink = $delegate[0].link.pre;
var origPostLink = $delegate[0].link.post;

$delegate[0].compile = function() {
return {
pre: origPreLink,
post: function() {
linkLog.push('linkNgOptions');
origPostLink.apply(this, arguments);
}
};
};

return $delegate;
});
}));

beforeEach(inject(function($rootScope, _$compile_) {
scope = $rootScope.$new(); //create a child scope because the root scope can't be $destroy-ed
$compile = _$compile_;
Expand Down Expand Up @@ -2119,6 +2166,35 @@ describe('ngOptions', function() {
option = element.find('option').eq(0);
expect(option.text()).toBe('A');
});


it('should not throw when a directive compiles the blank option before ngOptions is linked', function() {
expect(function() {
createSelect({
'o-compile-contents': '',
'name': 'select',
'ng-model': 'value',
'ng-options': 'item for item in items'
}, true);
}).not.toThrow();

expect(linkLog).toEqual(['linkCompileContents', 'linkNgOptions']);
});


it('should not throw with a directive that replaces', inject(function($templateCache, $httpBackend) {
$templateCache.put('select_template.html', '<select ng-options="option as option for option in selectable_options"> <option value="">This is a test</option> </select>');

scope.options = ['a', 'b', 'c', 'd'];

expect(function() {
element = $compile('<custom-select ng-model="value" options="options"></custom-select>')(scope);
scope.$digest();
}).not.toThrow();

dealoc(element);
}));

});


Expand Down