Skip to content

Commit

Permalink
perf($compile): don't register $destroy callbacks on element-transclu…
Browse files Browse the repository at this point in the history
…ded nodes

This is a major perf win in the large table benchmark (~100ms or 9).

This cleanup is needed only for regular transclusion because only then the DOM hierarchy doesn't match scope hierarchy
(transcluded scope is a child of the parent scope and not a child of the isolate scope)

We should consider refactoring this further for the case of regular transclusion
and consider using scope events instead.
  • Loading branch information
IgorMinar committed Aug 19, 2014
1 parent d05f27e commit b5f7970
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
9 changes: 6 additions & 3 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}

if ( nodeLinkFn.transcludeOnThisElement ) {
childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude, parentBoundTranscludeFn);
childBoundTranscludeFn = createBoundTranscludeFn(
scope, nodeLinkFn.transclude, parentBoundTranscludeFn,
nodeLinkFn.elementTranscludeOnThisElement);

} else if (!nodeLinkFn.templateOnThisElement && parentBoundTranscludeFn) {
childBoundTranscludeFn = parentBoundTranscludeFn;
Expand All @@ -1021,7 +1023,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
}

function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) {
function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn, elementTransclusion) {

var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) {
var scopeCreated = false;
Expand All @@ -1033,7 +1035,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}

var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn);
if (scopeCreated) {
if (scopeCreated && !elementTransclusion) {
clone.on('$destroy', function() { transcludedScope.$destroy(); });
}
return clone;
Expand Down Expand Up @@ -1410,6 +1412,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true;
nodeLinkFn.transcludeOnThisElement = hasTranscludeDirective;
nodeLinkFn.elementTranscludeOnThisElement = hasElementTranscludeDirective;
nodeLinkFn.templateOnThisElement = hasTemplate;
nodeLinkFn.transclude = childTranscludeFn;

Expand Down
6 changes: 3 additions & 3 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4128,16 +4128,16 @@ describe('$compile', function() {
function testCleanup() {
var privateData, firstRepeatedElem;

element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);
element = $compile('<div><div ng-repeat="x in xs" ng-click="noop()">{{x}}</div></div>')($rootScope);

$rootScope.$apply('xs = [' + xs + ']');
firstRepeatedElem = element.children('.ng-scope').eq(0);

expect(firstRepeatedElem.data('$scope')).toBeDefined();
privateData = jQuery._data(firstRepeatedElem[0]);
expect(privateData.events).toBeDefined();
expect(privateData.events.$destroy).toBeDefined();
expect(privateData.events.$destroy[0]).toBeDefined();
expect(privateData.events.click).toBeDefined();
expect(privateData.events.click[0]).toBeDefined();

$rootScope.$apply('xs = null');

Expand Down

0 comments on commit b5f7970

Please sign in to comment.