fix(ngTransclude): only compile fallback content if necessary

If the instance of the directive does provide transcluded content, then the fallback content should not be compiled and linked as it will never be used. If the instance of the directive does not provide transcluded content, then the transcluded scope that was created for this non-existent content is never used, so it should be destroy in order to clean up unwanted memory use and digests. Fixes #14768 Fixes #14765 Closes #14775
angular · Jun 17, 2016 · 159a68e · 159a68e
1 parent b9a56d5
commit 159a68e
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 30 deletions.
diff --git a/src/ng/directive/ngTransclude.js b/src/ng/directive/ngTransclude.js
@@ -159,35 +159,45 @@
  * </example>
  */
 var ngTranscludeMinErr = minErr('ngTransclude');
-var ngTranscludeDirective = ngDirective({
-  restrict: 'EAC',
-  link: function($scope, $element, $attrs, controller, $transclude) {
+var ngTranscludeDirective = ['$compile', function($compile) {
+  return {
+    restrict: 'EAC',
+    terminal: true,
+    link: function($scope, $element, $attrs, controller, $transclude) {
+      if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
+        // If the attribute is of the form: `ng-transclude="ng-transclude"`
+        // then treat it like the default
+        $attrs.ngTransclude = '';
+      }
 
-    if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
-      // If the attribute is of the form: `ng-transclude="ng-transclude"`
-      // then treat it like the default
-      $attrs.ngTransclude = '';
-    }
+      function ngTranscludeCloneAttachFn(clone, transcludedScope) {
+        if (clone.length) {
+          $element.empty();
+          $element.append(clone);
+        } else {
+          // Since this is the fallback content rather than the transcluded content,
+          // we compile against the scope we were linked against rather than the transcluded
+          // scope since this is the directive's own content
+          $compile($element.contents())($scope);
 
-    function ngTranscludeCloneAttachFn(clone) {
-      if (clone.length) {
-        $element.empty();
-        $element.append(clone);
+          // There is nothing linked against the transcluded scope since no content was available,
+          // so it should be safe to clean up the generated scope.
+          transcludedScope.$destroy();
+        }
       }
-    }
 
-    if (!$transclude) {
-      throw ngTranscludeMinErr('orphan',
-       'Illegal use of ngTransclude directive in the template! ' +
-       'No parent directive that requires a transclusion found. ' +
-       'Element: {0}',
-       startingTag($element));
-    }
-
-    // If there is no slot name defined or the slot name is not optional
-    // then transclude the slot
-    var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
-    $transclude(ngTranscludeCloneAttachFn, null, slotName);
-  }
-});
+      if (!$transclude) {
+        throw ngTranscludeMinErr('orphan',
+        'Illegal use of ngTransclude directive in the template! ' +
+        'No parent directive that requires a transclusion found. ' +
+        'Element: {0}',
+        startingTag($element));
+      }
 
+      // If there is no slot name defined or the slot name is not optional
+      // then transclude the slot
+      var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
+      $transclude(ngTranscludeCloneAttachFn, null, slotName);
+    }
+  };
+}];
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -7889,7 +7889,7 @@ describe('$compile', function() {
           directive('trans', function() {
             return {
               transclude: true,
-              template: '<div ng-transclude>old stuff! </div>'
+              template: '<div ng-transclude>old stuff!</div>'
             };
           });
         });
@@ -7906,18 +7906,76 @@ describe('$compile', function() {
           directive('trans', function() {
             return {
               transclude: true,
-              template: '<div ng-transclude>old stuff! </div>'
+              template: '<div ng-transclude>old stuff!</div>'
             };
           });
         });
         inject(function(log, $rootScope, $compile) {
           element = $compile('<div trans></div>')($rootScope);
           $rootScope.$apply();
-          expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">old stuff! </div>');
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""><span>old stuff!</span></div>');
         });
       });
 
 
+      it('should not compile the fallback content if transcluded content is provided', function() {
+        var contentsDidLink = false;
+
+        module(function() {
+          directive('inner', function() {
+            return {
+              restrict: 'E',
+              template: 'old stuff! ',
+              link: function() {
+                contentsDidLink = true;
+              }
+            };
+          });
+
+          directive('trans', function() {
+            return {
+              transclude: true,
+              template: '<div ng-transclude><inner></inner></div>'
+            };
+          });
+        });
+        inject(function($rootScope, $compile) {
+          element = $compile('<div trans>unicorn!</div>')($rootScope);
+          $rootScope.$apply();
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""><span>unicorn!</span></div>');
+          expect(contentsDidLink).toBe(false);
+        });
+      });
+
+      it('should compile and link the fallback content if no transcluded content is provided', function() {
+        var contentsDidLink = false;
+
+        module(function() {
+          directive('inner', function() {
+            return {
+              restrict: 'E',
+              template: 'old stuff! ',
+              link: function() {
+                contentsDidLink = true;
+              }
+            };
+          });
+
+          directive('trans', function() {
+            return {
+              transclude: true,
+              template: '<div ng-transclude><inner></inner></div>'
+            };
+          });
+        });
+        inject(function(log, $rootScope, $compile) {
+          element = $compile('<div trans></div>')($rootScope);
+          $rootScope.$apply();
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""><inner>old stuff! </inner></div>');
+          expect(contentsDidLink).toBe(true);
+        });
+      });
+
       it('should throw on an ng-transclude element inside no transclusion directive', function() {
         inject(function($rootScope, $compile) {
           // we need to do this because different browsers print empty attributes differently