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

perf(ngOptions): use documentFragment to populate select #14400

Merged
merged 1 commit into from
Apr 13, 2016
Merged

perf(ngOptions): use documentFragment to populate select #14400

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
134 changes: 39 additions & 95 deletions src/ng/directive/ngOptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ var NG_OPTIONS_REGEXP = /^\s*([\s\S]+?)(?:\s+as\s+([\s\S]+?))?(?:\s+group\s+by\s
// jshint maxlen: 100


var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile, $document, $parse) {

function parseOptionsExpression(optionsExp, selectElement, scope) {

Expand Down Expand Up @@ -432,7 +432,10 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {

var options;
var ngOptions = parseOptionsExpression(attr.ngOptions, selectElement, scope);

// This stores the newly created options before they are appended to the select.
// Since the contents are removed from the fragment when it is appended,
// we only need to create it once.
var listFragment = $document[0].createDocumentFragment();

var renderEmptyOption = function() {
if (!providedEmptyOption) {
Expand Down Expand Up @@ -581,6 +584,8 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
emptyOption = jqLite(optionTemplate.cloneNode(false));
}

selectElement.empty();

// We need to do this here to ensure that the options object is defined
// when we first hit it in writeNgOptionsValue
updateOptions();
Expand All @@ -590,6 +595,12 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {

// ------------------------------------------------------------------ //

function addOptionElement(option, parent) {
var optionElement = optionTemplate.cloneNode(false);
parent.appendChild(optionElement);
updateOptionElement(option, optionElement);
}


function updateOptionElement(option, element) {
option.element = element;
Expand All @@ -606,133 +617,66 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
if (option.value !== element.value) element.value = option.selectValue;
}

function addOrReuseElement(parent, current, type, templateElement) {
var element;
// Check whether we can reuse the next element
if (current && lowercase(current.nodeName) === type) {
// The next element is the right type so reuse it
element = current;
} else {
// The next element is not the right type so create a new one
element = templateElement.cloneNode(false);
if (!current) {
// There are no more elements so just append it to the select
parent.appendChild(element);
} else {
// The next element is not a group so insert the new one
parent.insertBefore(element, current);
}
}
return element;
}


function removeExcessElements(current) {
var next;
while (current) {
next = current.nextSibling;
jqLiteRemove(current);
current = next;
}
}

function updateOptions() {
var previousValue = options && selectCtrl.readValue();

function skipEmptyAndUnknownOptions(current) {
var emptyOption_ = emptyOption && emptyOption[0];
var unknownOption_ = unknownOption && unknownOption[0];

// We cannot rely on the extracted empty option being the same as the compiled empty option,
// because the compiled empty option might have been replaced by a comment because
// it had an "element" transclusion directive on it (such as ngIf)
if (emptyOption_ || unknownOption_) {
while (current &&
(current === emptyOption_ ||
current === unknownOption_ ||
current.nodeType === NODE_TYPE_COMMENT ||
(nodeName_(current) === 'option' && current.value === ''))) {
current = current.nextSibling;
// We must remove all current options, but cannot simply set innerHTML = null
// since the providedEmptyOption might have an ngIf on it that inserts comments which we
// must preserve.
// Instead, iterate over the current option elements and remove them or their optgroup
// parents
if (options) {

for (var i = options.items.length - 1; i >= 0; i--) {
var option = options.items[i];
if (option.group) {
jqLiteRemove(option.element.parentNode);
Copy link
Member

Choose a reason for hiding this comment

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

So, if there is a group, we don't need to preserve ngIf comments ?
How come ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I understand how this could happen. ngOption generated options can't have ngIf, and empty / unknown options are not part the options.items. So whatever we remove here will not touch any comments.

Copy link
Member

Choose a reason for hiding this comment

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

Also minor, but it seems that we will be calling jqLiteRemove() multiple times on the same optgroup element. Maybe we should avoid that.

Copy link
Member

Choose a reason for hiding this comment

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

I see about the ngIf. So badically only the empty option can have an ngIf. Makes sense.
There is a typo in the comment: providedOption --> providedEmptyOption 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't lookt like the muliple jqLiteRemove affects performance too much, so I would like to keep it like that for simplicity

} else {
jqLiteRemove(option.element);
}
}
}
return current;
}


function updateOptions() {

var previousValue = options && selectCtrl.readValue();

options = ngOptions.getOptions();

var groupMap = {};
var currentElement = selectElement[0].firstChild;
var groupElementMap = {};

// Ensure that the empty option is always there if it was explicitly provided
if (providedEmptyOption) {
selectElement.prepend(emptyOption);
}

currentElement = skipEmptyAndUnknownOptions(currentElement);

options.items.forEach(function updateOption(option) {
var group;
options.items.forEach(function addOption(option) {
var groupElement;
var optionElement;

if (isDefined(option.group)) {

// This option is to live in a group
// See if we have already created this group
group = groupMap[option.group];
groupElement = groupElementMap[option.group];

if (!group) {
if (!groupElement) {

// We have not already created this group
groupElement = addOrReuseElement(selectElement[0],
currentElement,
'optgroup',
optGroupTemplate);
// Move to the next element
currentElement = groupElement.nextSibling;
groupElement = optGroupTemplate.cloneNode(false);
listFragment.appendChild(groupElement);

// Update the label on the group element
groupElement.label = option.group;

// Store it for use later
group = groupMap[option.group] = {
groupElement: groupElement,
currentOptionElement: groupElement.firstChild
};

groupElementMap[option.group] = groupElement;
}

// So now we have a group for this option we add the option to the group
optionElement = addOrReuseElement(group.groupElement,
group.currentOptionElement,
'option',
optionTemplate);
updateOptionElement(option, optionElement);
// Move to the next element
group.currentOptionElement = optionElement.nextSibling;
addOptionElement(option, groupElement);

} else {

// This option is not in a group
optionElement = addOrReuseElement(selectElement[0],
currentElement,
'option',
optionTemplate);
updateOptionElement(option, optionElement);
// Move to the next element
currentElement = optionElement.nextSibling;
addOptionElement(option, listFragment);
}
});


// Now remove all excess options and group
Object.keys(groupMap).forEach(function(key) {
removeExcessElements(groupMap[key].currentOptionElement);
});
removeExcessElements(currentElement);
selectElement[0].appendChild(listFragment);

ngModelCtrl.$render();

Expand Down
2 changes: 2 additions & 0 deletions test/ng/directive/ngOptionsSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1946,6 +1946,8 @@ describe('ngOptions', function() {
scope.options[1].unavailable = false;
});

options = element.find('option');

expect(scope.options[1].unavailable).toEqual(false);
expect(options.eq(1).prop('disabled')).toEqual(false);
});
Expand Down