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

Commit

Permalink
perf(jqLite): implement and use the empty method in place of `html(…
Browse files Browse the repository at this point in the history
…‘’)`

jQuery's elem.html('') is way slower than elem.empty(). As clearing
element contents happens quite often in certain scenarios, switching
to using .empty() provides a significant performance boost when using
Angular with jQuery.

Closes #4457
  • Loading branch information
mgol authored and IgorMinar committed Dec 13, 2013
1 parent f3de5b6 commit 3410f65
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 30 deletions.
2 changes: 1 addition & 1 deletion docs/component-spec/annotationsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ describe('Docs Annotations', function() {
var body;
beforeEach(function() {
body = angular.element(document.body);
body.html('');
body.empty();
});

var normalizeHtml = function(html) {
Expand Down
2 changes: 1 addition & 1 deletion docs/components/angular-bootstrap/bootstrap-prettify.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function escape(text) {
function setHtmlIe8SafeWay(element, html) {
var newElement = angular.element('<pre>' + html + '</pre>');

element.html('');
element.empty();
element.append(newElement.contents());
return element;
}
Expand Down
2 changes: 1 addition & 1 deletion docs/content/guide/dev_guide.unit-testing.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ var pc = new PasswordCtrl();
input.val('abc');
pc.grade();
expect(span.text()).toEqual('weak');
$('body').html('');
$('body').empty();
</pre>

In angular the controllers are strictly separated from the DOM manipulation logic and this results in
Expand Down
2 changes: 1 addition & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ function startingTag(element) {
try {
// turns out IE does not let you set .html() on elements which
// are not allowed to have children. So we just ignore it.
element.html('');
element.empty();
} catch(e) {}
// As Per DOM Standards
var TEXT_NODE = 3;
Expand Down
22 changes: 18 additions & 4 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
* - [`contents()`](http://api.jquery.com/contents/)
* - [`css()`](http://api.jquery.com/css/)
* - [`data()`](http://api.jquery.com/data/)
* - [`empty()`](http://api.jquery.com/empty/)
* - [`eq()`](http://api.jquery.com/eq/)
* - [`find()`](http://api.jquery.com/find/) - Limited to lookups by tag name
* - [`hasClass()`](http://api.jquery.com/hasClass/)
Expand Down Expand Up @@ -358,6 +359,15 @@ function jqLiteInheritedData(element, name, value) {
}
}

function jqLiteEmpty(element) {
for (var i = 0, childNodes = element.childNodes; i < childNodes.length; i++) {
jqLiteDealoc(childNodes[i]);
}
while (element.firstChild) {
element.removeChild(element.firstChild);
}
}

//////////////////////////////////////////
// Functions which are declared directly.
//////////////////////////////////////////
Expand Down Expand Up @@ -552,7 +562,9 @@ forEach({
jqLiteDealoc(childNodes[i]);
}
element.innerHTML = value;
}
},

empty: jqLiteEmpty
}, function(fn, name){
/**
* Properties: writes return selection, reads return first value
Expand All @@ -562,11 +574,13 @@ forEach({

// jqLiteHasClass has only two arguments, but is a getter-only fn, so we need to special-case it
// in a way that survives minification.
if (((fn.length == 2 && (fn !== jqLiteHasClass && fn !== jqLiteController)) ? arg1 : arg2) === undefined) {
// jqLiteEmpty takes no arguments but is a setter.
if (fn !== jqLiteEmpty &&
(((fn.length == 2 && (fn !== jqLiteHasClass && fn !== jqLiteController)) ? arg1 : arg2) === undefined)) {
if (isObject(arg1)) {

// we are a write, but the object properties are the key/values
for(i=0; i < this.length; i++) {
for (i = 0; i < this.length; i++) {
if (fn === jqLiteData) {
// data() takes the whole object in jQuery
fn(this[i], arg1);
Expand All @@ -591,7 +605,7 @@ forEach({
}
} else {
// we are a write, so apply to all children
for(i=0; i < this.length; i++) {
for (i = 0; i < this.length; i++) {
fn(this[i], arg1, arg2);
}
// return self for chaining
Expand Down
4 changes: 2 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
});
} else {
$template = jqLite(jqLiteClone(compileNode)).contents();
$compileNode.html(''); // clear contents
$compileNode.empty(); // clear contents
childTranscludeFn = compile($template, transcludeFn);
}
}
Expand Down Expand Up @@ -1651,7 +1651,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
? origAsyncDirective.templateUrl($compileNode, tAttrs)
: origAsyncDirective.templateUrl;

$compileNode.html('');
$compileNode.empty();

$http.get($sce.getTrustedResourceUrl(templateUrl), {cache: $templateCache}).
success(function(content) {
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/ngTransclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var ngTranscludeDirective = ngDirective({

link: function($scope, $element, $attrs, controller) {
controller.$transclude(function(clone) {
$element.html('');
$element.empty();
$element.append(clone);
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,13 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
// becomes the compilation root
nullOption.removeClass('ng-scope');

// we need to remove it before calling selectElement.html('') because otherwise IE will
// we need to remove it before calling selectElement.empty() because otherwise IE will
// remove the label from the element. wtf?
nullOption.remove();
}

// clear contents, we'll add what's needed based on the model
selectElement.html('');
selectElement.empty();

selectElement.on('change', function() {
scope.$apply(function() {
Expand Down
2 changes: 1 addition & 1 deletion src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ angular.module('ngAnimate', ['ng'])
//make the element super hidden and override any CSS style values
clone.attr('style','position:absolute; top:-9999px; left:-9999px');
clone.removeAttr('id');
clone.html('');
clone.empty();

forEach(oldClasses.split(' '), function(klass) {
clone.removeClass(klass);
Expand Down
3 changes: 1 addition & 2 deletions test/helpers/testabilityPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ beforeEach(function() {
bindJQuery();
}


angular.element(document.body).html('').removeData();
angular.element(document.body).empty().removeData();
});

afterEach(function() {
Expand Down
25 changes: 23 additions & 2 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ describe('jqLite', function() {
});


it('should emit $destroy event if an element is removed via html()', inject(function(log) {
it('should emit $destroy event if an element is removed via html(\'\')', inject(function(log) {
var element = jqLite('<div><span>x</span></div>');
element.find('span').on('$destroy', log.fn('destroyed'));

Expand All @@ -333,6 +333,17 @@ describe('jqLite', function() {
}));


it('should emit $destroy event if an element is removed via empty()', inject(function(log) {
var element = jqLite('<div><span>x</span></div>');
element.find('span').on('$destroy', log.fn('destroyed'));

element.empty();

expect(element.html()).toBe('');
expect(log).toEqual('destroyed');
}));


it('should retrieve all data if called without params', function() {
var element = jqLite(a);
expect(element.data()).toEqual({});
Expand Down Expand Up @@ -786,7 +797,7 @@ describe('jqLite', function() {
});


it('should read/write value', function() {
it('should read/write a value', function() {
var element = jqLite('<div>abc</div>');
expect(element.length).toEqual(1);
expect(element[0].innerHTML).toEqual('abc');
Expand All @@ -797,6 +808,16 @@ describe('jqLite', function() {
});


describe('empty', function() {
it('should write a value', function() {
var element = jqLite('<div>abc</div>');
expect(element.length).toEqual(1);
expect(element.empty() == element).toBeTruthy();
expect(element.html()).toEqual('');
});
});


describe('on', function() {
it('should bind to window on hashchange', function() {
if (jqLite.fn) return; // don't run in jQuery
Expand Down
8 changes: 4 additions & 4 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,26 +170,26 @@ describe('$compile', function() {
// First with only elements at the top level
element = jqLite('<div><div></div></div>');
$compile(element.contents())($rootScope);
element.html('');
element.empty();
expect(calcCacheSize()).toEqual(0);

// Next with non-empty text nodes at the top level
// (in this case the compiler will wrap them in a <span>)
element = jqLite('<div>xxx</div>');
$compile(element.contents())($rootScope);
element.html('');
element.empty();
expect(calcCacheSize()).toEqual(0);

// Next with comment nodes at the top level
element = jqLite('<div><!-- comment --></div>');
$compile(element.contents())($rootScope);
element.html('');
element.empty();
expect(calcCacheSize()).toEqual(0);

// Finally with empty text nodes at the top level
element = jqLite('<div> \n<div></div> </div>');
$compile(element.contents())($rootScope);
element.html('');
element.empty();
expect(calcCacheSize()).toEqual(0);
});

Expand Down
2 changes: 1 addition & 1 deletion test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ describe('form', function() {
// yes, I know, scope methods should not do direct DOM manipulation, but I wanted to keep
// this test small. Imagine that the destroy action will cause a model change (e.g.
// $location change) that will cause some directive to destroy the dom (e.g. ngView+$route)
doc.html('');
doc.empty();
destroyed = true;
}

Expand Down
8 changes: 4 additions & 4 deletions test/ng/directive/ngIncludeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('ngInclude', function() {
$rootScope.url = 'myUrl';
$rootScope.$digest();
expect(body.text()).toEqual('misko');
body.html('');
body.empty();
}));


Expand All @@ -60,7 +60,7 @@ describe('ngInclude', function() {
$rootScope.url = 'myUrl';
$rootScope.$digest();
expect(element.text()).toEqual('Alibaba');
jqLite(document.body).html('');
jqLite(document.body).empty();
}));


Expand All @@ -74,7 +74,7 @@ describe('ngInclude', function() {
expect(function() { $rootScope.$digest(); }).toThrowMinErr(
'$sce', 'insecurl',
/Blocked loading resource from url not allowed by \$sceDelegate policy. URL: http:\/\/example.com\/myUrl.*/);
jqLite(document.body).html('');
jqLite(document.body).empty();
}));


Expand All @@ -88,7 +88,7 @@ describe('ngInclude', function() {
expect(function() { $rootScope.$digest(); }).toThrowMinErr(
'$sce', 'insecurl',
/Blocked loading resource from url not allowed by \$sceDelegate policy. URL: http:\/\/example.com\/myUrl.*/);
jqLite(document.body).html('');
jqLite(document.body).empty();
}));


Expand Down
2 changes: 1 addition & 1 deletion test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ describe("ngAnimate", function() {
inject(function($animate, $compile, $rootScope, $timeout, $sniffer) {

$rootScope.$digest();
element.html('');
element.empty();

var child1 = $compile('<div>1</div>')($rootScope);
var child2 = $compile('<div>2</div>')($rootScope);
Expand Down
2 changes: 1 addition & 1 deletion test/ngScenario/dslSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("angular.scenario.dsl", function() {
// Just use the real one since it delegates to this.addFuture
$root.addFutureAction = angular.scenario.
SpecRunner.prototype.addFutureAction;
jqLite($window.document).html('');
jqLite($window.document).empty();
}));

afterEach(function(){
Expand Down
2 changes: 1 addition & 1 deletion test/ngTouch/directive/ngClickSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('ngClick (touch)', function() {
}));

afterEach(inject(function($document) {
$document.find('body').html('');
$document.find('body').empty();
}));


Expand Down

0 comments on commit 3410f65

Please sign in to comment.