Skip to content
This repository was 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
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
@@ -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];
@@ -448,7 +444,6 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
unknownOption.remove();
};


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

@@ -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
@@ -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;
};
}];

/**
@@ -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];
@@ -378,7 +393,6 @@ var selectDirective = function() {

}
}
};
};


@@ -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
78 changes: 77 additions & 1 deletion test/ng/directive/ngOptionsSpec.js
Original file line number Diff line number Diff line change
@@ -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>');
@@ -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_;
@@ -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);
}));

});