fix($compile): swap keys and values for transclude definition object

petebacondarwin · petebacondarwin · commit c3a2691115b9 · 2015-12-04T21:23:38.000Z
Closes angular#13439
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -237,7 +237,8 @@
  *          as those elements need to created and cloned in a special way when they are defined outside their
  *          usual containers (e.g. like `<svg>`).
  *        * See also the `directive.templateNamespace` property.
- *
+ *    The `$transclude` function has a property called `$slots`, which is a hash of slot names to slot transclusion
+ *    functions. If a slot was declared but not filled its value on the `$slots` object will be `null`.
  *
  * #### `require`
  * Require another directive and inject its controller as the fourth argument to the linking function. The
@@ -337,14 +338,22 @@
  * The contents are compiled and provided to the directive as a **transclusion function**. See the
  * {@link $compile#transclusion Transclusion} section below.
  *
- * There are two kinds of transclusion depending upon whether you want to transclude just the contents of the
- * directive's element or the entire element:
+ * There are three kinds of transclusion depending upon whether you want to transclude just the contents of the
+ * directive's element, the entire element or parts of the element:
  *
  * * `true` - transclude the content (i.e. the child nodes) of the directive's element.
  * * `'element'` - transclude the whole of the directive's element including any directives on this
  *   element that defined at a lower priority than this directive. When used, the `template`
  *   property is ignored.
+ * * **`{...}` (an object hash):** - map elements of the content onto transclusion "slots" in the template.
+ *   See {@link ngTransclude} for more information.
+ *
+ * Mult-slot transclusion is declared by providing an object for the `transclude` property.
+ * This object is a map where the keys are the canonical name of HTML elements to match in the transcluded HTML,
+ * and the values are the names of the slots. If the name is prefixed with a `?` then that slot is optional.
  *
+ * The slots are made available as `$transclude.$slots` on the transclude function that is passed to the
+ * linking functions as the fifth parameter, and can be injected into the directive controller.
  *
  * #### `compile`
  *
@@ -1511,7 +1520,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
       // so that they are available inside the `controllersBoundTransclude` function
       var boundSlots = boundTranscludeFn.$$slots = createMap();
       for (var slotName in transcludeFn.$$slots) {
-        boundSlots[slotName] = createBoundTranscludeFn(scope, transcludeFn.$$slots[slotName], previousBoundTranscludeFn);
+        if (transcludeFn.$$slots[slotName]) {
+          boundSlots[slotName] = createBoundTranscludeFn(scope, transcludeFn.$$slots[slotName], previousBoundTranscludeFn);
+        } else {
+          boundSlots[slotName] = null;
+        }
       }
 
       return boundTranscludeFn;
@@ -1855,32 +1868,42 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           } else {
 
             var slots = createMap();
+
             $template = jqLite(jqLiteClone(compileNode)).contents();
 
             if (isObject(directiveValue)) {
 
-              // We have transclusion slots - collect them up and compile them and store their
-              // transclusion functions
+              // We have transclusion slots,
+              // collect them up, compile them and store their transclusion functions
               $template = [];
-              var slotNames = createMap();
+
+              var slotMap = createMap();
               var filledSlots = createMap();
 
-              // Parse the slot names: if they start with a ? then they are optional
-              forEach(directiveValue, function(slotName, key) {
-                var optional = (slotName.charAt(0) === '?');
-                slotName = optional ? slotName.substring(1) : slotName;
-                slotNames[key] = slotName;
-                slots[slotName] = [];
+              // Parse the element selectors
+              forEach(directiveValue, function(elementSelector, slotName) {
+                // If an element selector starts with a ? then it is optional
+                var optional = (elementSelector.charAt(0) === '?');
+                elementSelector = optional ? elementSelector.substring(1) : elementSelector;
+
+                slotMap[elementSelector] = slotName;
+
+                // We explicitly assign `null` since this implies that a slot was defined but not filled.
+                // Later when calling boundTransclusion functions with a slot name we only error if the
+                // slot is `undefined`
+                slots[slotName] = null;
+
                 // filledSlots contains `true` for all slots that are either optional or have been
                 // filled. This is used to check that we have not missed any required slots
                 filledSlots[slotName] = optional;
               });
 
               // Add the matching elements into their slot
               forEach($compileNode.contents(), function(node) {
-                var slotName = slotNames[directiveNormalize(nodeName_(node))];
+                var slotName = slotMap[nodeName_(node)];
                 if (slotName) {
                   filledSlots[slotName] = true;
+                  slots[slotName] = slots[slotName] || [];
                   slots[slotName].push(node);
                 } else {
                   $template.push(node);
@@ -1894,9 +1917,12 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
                 }
               });
 
-              forEach(Object.keys(slots), function(slotName) {
-                slots[slotName] = compilationGenerator(mightHaveMultipleTransclusionError, slots[slotName], transcludeFn);
-              });
+              for (var slotName in slots) {
+                if (slots[slotName]) {
+                  // Only define a transclusion function if the slot was filled
+                  slots[slotName] = compilationGenerator(mightHaveMultipleTransclusionError, slots[slotName], transcludeFn);
+                }
+              }
             }
 
             $compileNode.empty(); // clear contents
@@ -2125,6 +2151,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           // is later passed as `parentBoundTranscludeFn` to `publicLinkFn`
           transcludeFn = controllersBoundTransclude;
           transcludeFn.$$boundTransclude = boundTranscludeFn;
+          // expose the slots on the `$transclude` function
+          transcludeFn.$slots = boundTranscludeFn.$$slots;
         }
 
         if (controllerDirectives) {
@@ -2221,16 +2249,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
             futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element;
           }
           if (slotName) {
+            // slotTranscludeFn can be one of three things:
+            //  * a transclude function - a filled slot
+            //  * `null` - an optional slot that was not filled
+            //  * `undefined` - a slot that was not declared (i.e. invalid)
             var slotTranscludeFn = boundTranscludeFn.$$slots[slotName];
-            if (!slotTranscludeFn) {
+            if (slotTranscludeFn) {
+              return slotTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
+            } else if (isUndefined(slotTranscludeFn)) {
               throw $compileMinErr('noslot',
                'No parent directive that requires a transclusion with slot name "{0}". ' +
                'Element: {1}',
                slotName, startingTag($element));
             }
-            return slotTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
+          } else {
+            return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
           }
-          return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
         }
       }
     }
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -7660,7 +7660,7 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              boss: 'bossSlot'
+              bossSlot: 'boss'
             },
             template:
               '<div class="other" ng-transclude></div>'
@@ -7722,7 +7722,7 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              boss: 'bossSlot'
+              bossSlot: 'boss'
             },
             template:
               '<div class="other" ng-transclude></div>'
@@ -7751,8 +7751,8 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot',
-              boss: 'bossSlot'
+              minionSlot: 'minion',
+              bossSlot: 'boss'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7784,8 +7784,8 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot',
-              boss: 'bossSlot'
+              minionSlot: 'minion',
+              bossSlot: 'boss'
             },
             template:
               '<ng-transclude class="boss" ng-transclude-slot="bossSlot"></ng-transclude>' +
@@ -7816,8 +7816,8 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot',
-              boss: 'bossSlot'
+              minionSlot: 'minion',
+              bossSlot: 'boss'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7845,8 +7845,8 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot',
-              boss: '?bossSlot'
+              minionSlot: 'minion',
+              bossSlot: '?boss'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7875,7 +7875,7 @@ describe('$compile', function() {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot'
+              minionSlot: 'minion'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7923,14 +7923,14 @@ describe('$compile', function() {
     });
 
 
-    it('should match against the normalized form of the element', function() {
+    it('should not normalize the element name', function() {
       module(function() {
         directive('foo', function() {
           return {
             restrict: 'E',
             scope: {},
             transclude: {
-              fooBar: 'fooBarSlot'
+              fooBarSlot: 'foo-bar'
             },
             template:
               '<div class="other" ng-transclude="fooBarSlot"></div>'
@@ -7948,16 +7948,16 @@ describe('$compile', function() {
     });
 
 
-    it('should provide the elements marked with matching transclude elements as additional transclude functions on the $$slots property', function() {
+    it('should provide the elements marked with matching transclude elements as additional transclude functions on the $slots property', function() {
       var capturedTranscludeFn;
       module(function() {
         directive('minionComponent', function() {
           return {
             restrict: 'E',
             scope: {},
             transclude: {
-              minion: 'minionSlot',
-              boss: 'bossSlot'
+              minionSlot: 'minion',
+              bossSlot: 'boss'
             },
             template:
               '<div class="boss" ng-transclude="bossSlot"></div>' +
@@ -7979,7 +7979,7 @@ describe('$compile', function() {
           '</minion-component>')($rootScope);
         $rootScope.$apply();
 
-        var minionTranscludeFn = capturedTranscludeFn.$$boundTransclude.$$slots['minionSlot'];
+        var minionTranscludeFn = capturedTranscludeFn.$slots['minionSlot'];
         var minions = minionTranscludeFn();
         expect(minions[0].outerHTML).toEqual('<minion class="ng-scope">stuart</minion>');
         expect(minions[1].outerHTML).toEqual('<minion class="ng-scope">bob</minion>');
@@ -7989,7 +7989,7 @@ describe('$compile', function() {
         var minionScope = jqLite(minions[0]).scope();
         expect(minionScope.$parent).toBe(scope);
 
-        var bossTranscludeFn = capturedTranscludeFn.$$boundTransclude.$$slots['bossSlot'];
+        var bossTranscludeFn = capturedTranscludeFn.$slots['bossSlot'];
         var boss = bossTranscludeFn();
         expect(boss[0].outerHTML).toEqual('<boss class="ng-scope">gru</boss>');
 
@@ -8002,6 +8002,34 @@ describe('$compile', function() {
         dealoc(minions);
       });
     });
+
+    it('should set unfilled optional transclude slots to `null` in the $transclude.$slots property', function() {
+      var capturedTranscludeFn;
+      module(function() {
+        directive('minionComponent', function() {
+          return {
+            restrict: 'E',
+            scope: {},
+            transclude: {
+              minionSlot: 'minion',
+              bossSlot: '?boss'
+            },
+            link: function(s, e, a, c, transcludeFn) {
+              capturedTranscludeFn = transcludeFn;
+            }
+          };
+        });
+      });
+      inject(function($rootScope, $compile) {
+        element = $compile(
+          '<minion-component>' +
+            '<minion>stuart</minion>' +
+            '<span>dorothy</span>' +
+          '</minion-component>')($rootScope);
+        expect(capturedTranscludeFn.$slots.minionSlot).toEqual(jasmine.any(Function));
+        expect(capturedTranscludeFn.$slots.bossSlot).toBe(null);
+      });
+    });
   });
 
 
