WIP - DO NOT MERGE - feat($compile): multiple transclusion via slots

[petebacondarwin]

This is a really rough and ready spike of multiple transclusion via slots.

There are LOTS of issues, primarily around the way that transclude functions are created, wrapped and passed around often hidden inside closures with access to a variety of other variables.
It is also notable that the transclude function, that gets passed to the directive's controller and link functions, is incompatible with the transclude functions (even the bound ones) that are passed around internally, since the former allows the scope parameter to be missing.
For this to be implemented properly I fear we may need to completely refactor large parts of the way transclusion is passed around inside the compiler.

Also it doesn't currently work for element transclusion.

This is an alternative strategy to #11736

[petebacondarwin]

@kara and @IgorMinar do you want to take a quick look?

[jbedard]

Should this be accessible on the public $transclude function for cases where ng-transclude is not used? $transclude.slots.name maybe? I feel like this $compile feature should not be created only for ng-transclude...

[petebacondarwin]

I agree. At this stage it is available on the transclude function via the convoluted:

$transclude.$$boundTransclude.$$slots[name]

See https://github.com/angular/angular.js/pull/12742/files#diff-348c2f3781ed66a24894c2046a52c628R7114

But it should be exposed more cleanly

[IgorMinar]

why is the first argument undefined?

[IgorMinar]

oh, I see your comments in the PR description.

[IgorMinar]

This actually looks pretty reasonable. It would be great if we could complain about any slots defined in the transcluded template (light dom) but not in the transcluding template (shadow dom).

I don't think we need to support element transclusion. I don't think that it makes to mix that with slots.

Could we get away with not supporting imperative access to the transclusion function (via directive controller or link fn)?

[petebacondarwin]

@IgorMinar can you clarify what you mean by this?

Could we get away with not supporting imperative access to the transclusion function (via directive controller or link fn)?

[fenduru]

I am a huge fan of the concept here, but I would like to voice my concern about the API. I work on a project where we build reusable components for other teams at my company. A typical component might look something like this:

<my-component>
  <my-component-header>Header</my-component-header>
  <my-component-content>Content</my-component-content>
  <my-component-footer>Footer</my-component-footer>
</my-component>

The nice thing about this is that during an initial implementation, myComponent might not even use transclusion, and this is an implementation detail completely hidden from my consumers. Then when new features are added in that require transclusion, this does not force me to make a breaking change to my API.

TL;DR please don't make me (reusable component author) export implementation detail as part of my API

FWIW I accomplished the API that I've described here by using $$TLB + transclude: element to re-define the child components, require the known parent, and transclude itself into the correct outlet. Usage looks like this (actual usage is easier because I've written some helpers):

module.directive('parentComponent', function(multiTranscludeController) {
  return {

    // Must pass the parent controller so that the slot registers with the correct controller even when there are multiple levels of nesting
    // Default slot is needed so that the children get linked
    template: '<transclude-slot name="default" parent="ctrl"></transclude-slot>' +
                    '<transclude-slot name="childGoesHere" parent="ctrl"></transclude-slot>',

    // This controller contains functions used to register both slots and children with the parent
    controller: multiTranscludeController,
    controllerAs: 'ctrl'
  }
})

.directive('childComponent', function() {
  // normal directive definition, do whatever you want here (including your own transclusion)
})

.directive('childComponent', function() {
  return {
    transclude: 'element',
    $$tlb: true,
    priority: 500,
    requre: '^^parentComponent',
    link: function() {
      // Register with the parent controller
    }
  };
})

[petebacondarwin]

@fenduru I hear that you are saying that you want to have control over the API surface that your directive exposes to its clients.

I was initially nervous about this since it means that each directive must document what gets transcluded, and that users of directives must understand each directive's use of transclusion. In the way that is implemented in this PR users of all directives would just need to learn one rule which is about ng-slot. That being said, I am coming round to the idea of custom slot identifiers. Apart from anything else it is not clear in any case in a directive what slot names are required.

What do you think of the possible implementation described below:

[petebacondarwin]

Define the transclusion slots via the transclude property on the directive definition object. So instead of only true or element you can provide a hash object:

{
  ...
  tranclude: {
    elementName1: 'transcludeSlot1',
    elementName2: '?optionalTranscludeSlot2',
    ...
  },
  ...
}

The keys match elements of the transcluded HTML that should be extracted into a slot.
The values match the names of the transclude slots, with a prefix of ? indicating that the slot is optional.

This would allow the directive designer to provide their own API for transclusion slots and also for the compiler to indicate an error if non-optional slots are not provided by the directive user.

So the usage would be:

<some-directive>
  <element-name1>... required element with content to be transcluded ...</element-name1>
  <element-name2>... this element is optional ...</element-name2>
</some-directive>

Internally the directive template would continue to use ng-transclude="slotName".

[fenduru]

@petebacondarwin That approach basically sounds like syntactic sugar for something like:

{
  transclude: true,
  link: function(scope, elem, attrs, ctrl, transclude) {
    transclude(function(clone) {
      clone.find('elementName1').appendTo(elem.find('transcludeSlot1'));
      clone.find('elementName2').appendTo(elem.find('transcludeSlot2'));
    });
  }
}

This approach is very naive IMO. It breaks down very quickly when you introduce things that have their own transclusion. For instance, what happens when you put ngIf on the optional elementName1? I would expect it to end up in the transclusion slot when the condition turns true, but this would not happen. Another case is when using ngInclude which is pretty much the only way to do recursive directives until #12078 lands.

I've spent a lot of time working on this class of problems, and I'm fairly confident that the only point where it is safe to handle this transclusion behavior is in the child elements link function. This is the only point where we know for sure that the child exists, and what it is inside of.

[petebacondarwin]

What I suggested is not syntactic sugar for what you suggested. The implementation would be similar to what is in this PR.

[IgorMinar]

@petebacondarwin I like the idea of specifying the map in DDO, would that help to solve the issue of accessing the transclusion functions for individual slots from within the linking function?

[petebacondarwin]

I will create an alternative PR for this.

[petebacondarwin]

Even though this is not the final solution, I rebased it due to lazy compilation.