From 3410f65e790a81d457b4f4601a1e760a6f8ede5e Mon Sep 17 00:00:00 2001 From: Michał Gołębiowski Date: Wed, 16 Oct 2013 15:15:21 +0200 Subject: perf(jqLite): implement and use the `empty` method in place of `html(‘’)` 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 --- docs/component-spec/annotationsSpec.js | 2 +- .../angular-bootstrap/bootstrap-prettify.js | 2 +- docs/content/guide/dev_guide.unit-testing.ngdoc | 2 +- src/Angular.js | 2 +- src/jqLite.js | 22 +++++++++++++++---- src/ng/compile.js | 4 ++-- src/ng/directive/ngTransclude.js | 2 +- src/ng/directive/select.js | 4 ++-- src/ngAnimate/animate.js | 2 +- test/helpers/testabilityPatch.js | 3 +-- test/jqLiteSpec.js | 25 ++++++++++++++++++++-- test/ng/compileSpec.js | 8 +++---- test/ng/directive/formSpec.js | 2 +- test/ng/directive/ngIncludeSpec.js | 8 +++---- test/ngAnimate/animateSpec.js | 2 +- test/ngScenario/dslSpec.js | 2 +- test/ngTouch/directive/ngClickSpec.js | 2 +- 17 files changed, 64 insertions(+), 30 deletions(-) diff --git a/docs/component-spec/annotationsSpec.js b/docs/component-spec/annotationsSpec.js index a2a78077..60b17d9a 100644 --- a/docs/component-spec/annotationsSpec.js +++ b/docs/component-spec/annotationsSpec.js @@ -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) { diff --git a/docs/components/angular-bootstrap/bootstrap-prettify.js b/docs/components/angular-bootstrap/bootstrap-prettify.js index 43248943..72c136d1 100644 --- a/docs/components/angular-bootstrap/bootstrap-prettify.js +++ b/docs/components/angular-bootstrap/bootstrap-prettify.js @@ -28,7 +28,7 @@ function escape(text) { function setHtmlIe8SafeWay(element, html) { var newElement = angular.element('
' + html + '
'); - element.html(''); + element.empty(); element.append(newElement.contents()); return element; } diff --git a/docs/content/guide/dev_guide.unit-testing.ngdoc b/docs/content/guide/dev_guide.unit-testing.ngdoc index 9d3d9765..e15edeb7 100644 --- a/docs/content/guide/dev_guide.unit-testing.ngdoc +++ b/docs/content/guide/dev_guide.unit-testing.ngdoc @@ -222,7 +222,7 @@ var pc = new PasswordCtrl(); input.val('abc'); pc.grade(); expect(span.text()).toEqual('weak'); -$('body').html(''); +$('body').empty(); In angular the controllers are strictly separated from the DOM manipulation logic and this results in diff --git a/src/Angular.js b/src/Angular.js index b09d3a7f..e32c3739 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -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; diff --git a/src/jqLite.js b/src/jqLite.js index e7531fb3..8a45f966 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -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/) @@ -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. ////////////////////////////////////////// @@ -552,7 +562,9 @@ forEach({ jqLiteDealoc(childNodes[i]); } element.innerHTML = value; - } + }, + + empty: jqLiteEmpty }, function(fn, name){ /** * Properties: writes return selection, reads return first value @@ -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); @@ -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 diff --git a/src/ng/compile.js b/src/ng/compile.js index a6bcc3e7..0af76a75 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -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); } } @@ -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) { diff --git a/src/ng/directive/ngTransclude.js b/src/ng/directive/ngTransclude.js index 490ea21f..8eefb6ff 100644 --- a/src/ng/directive/ngTransclude.js +++ b/src/ng/directive/ngTransclude.js @@ -69,7 +69,7 @@ var ngTranscludeDirective = ngDirective({ link: function($scope, $element, $attrs, controller) { controller.$transclude(function(clone) { - $element.html(''); + $element.empty(); $element.append(clone); }); } diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 86e04242..d87fa5d3 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -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() { diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 8ff7b429..aeb6e32e 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -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); diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js index 41b5042a..f61be4ee 100644 --- a/test/helpers/testabilityPatch.js +++ b/test/helpers/testabilityPatch.js @@ -29,8 +29,7 @@ beforeEach(function() { bindJQuery(); } - - angular.element(document.body).html('').removeData(); + angular.element(document.body).empty().removeData(); }); afterEach(function() { diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 09be1c1c..c4f47dcd 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -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('
x
'); element.find('span').on('$destroy', log.fn('destroyed')); @@ -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('
x
'); + 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({}); @@ -786,7 +797,7 @@ describe('jqLite', function() { }); - it('should read/write value', function() { + it('should read/write a value', function() { var element = jqLite('
abc
'); expect(element.length).toEqual(1); expect(element[0].innerHTML).toEqual('abc'); @@ -797,6 +808,16 @@ describe('jqLite', function() { }); + describe('empty', function() { + it('should write a value', function() { + var element = jqLite('
abc
'); + 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 diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 853290a6..1b98cd58 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -170,26 +170,26 @@ describe('$compile', function() { // First with only elements at the top level element = jqLite('
'); $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 ) element = jqLite('
xxx
'); $compile(element.contents())($rootScope); - element.html(''); + element.empty(); expect(calcCacheSize()).toEqual(0); // Next with comment nodes at the top level element = jqLite('
'); $compile(element.contents())($rootScope); - element.html(''); + element.empty(); expect(calcCacheSize()).toEqual(0); // Finally with empty text nodes at the top level element = jqLite('
\n
'); $compile(element.contents())($rootScope); - element.html(''); + element.empty(); expect(calcCacheSize()).toEqual(0); }); diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index 77beb2fd..dde6f0a0 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -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; } diff --git a/test/ng/directive/ngIncludeSpec.js b/test/ng/directive/ngIncludeSpec.js index 9e0a47b0..79e7f312 100644 --- a/test/ng/directive/ngIncludeSpec.js +++ b/test/ng/directive/ngIncludeSpec.js @@ -47,7 +47,7 @@ describe('ngInclude', function() { $rootScope.url = 'myUrl'; $rootScope.$digest(); expect(body.text()).toEqual('misko'); - body.html(''); + body.empty(); })); @@ -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(); })); @@ -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(); })); @@ -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(); })); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 44b623b4..01589da1 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -301,7 +301,7 @@ describe("ngAnimate", function() { inject(function($animate, $compile, $rootScope, $timeout, $sniffer) { $rootScope.$digest(); - element.html(''); + element.empty(); var child1 = $compile('
1
')($rootScope); var child2 = $compile('
2
')($rootScope); diff --git a/test/ngScenario/dslSpec.js b/test/ngScenario/dslSpec.js index f94ec583..2553fadd 100644 --- a/test/ngScenario/dslSpec.js +++ b/test/ngScenario/dslSpec.js @@ -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(){ diff --git a/test/ngTouch/directive/ngClickSpec.js b/test/ngTouch/directive/ngClickSpec.js index 0eccc8f3..43735709 100644 --- a/test/ngTouch/directive/ngClickSpec.js +++ b/test/ngTouch/directive/ngClickSpec.js @@ -152,7 +152,7 @@ describe('ngClick (touch)', function() { })); afterEach(inject(function($document) { - $document.find('body').html(''); + $document.find('body').empty(); })); -- cgit v1.2.3