Transclusion bug(?): detached nodes get linked

[Wizek]

http://jsbin.com/yoqowaha/6/edit

What happens

Both elements get linked resulting in 1 and 2 both being output to the console.

Expected behavior

Only appended nodes should get linked therefore the console should contain only 1.

Background

Further details on my motivation (to prevent an XY problem) and how I stumbled into this issue with @caitp on IRC:

11:39 (Wizek) Hi guys!
11:42 (Wizek) Anyone here familiar with the $transclude function?
11:42 caitp-: ask your question
11:42 caitp-: receive an answer from the oracle
11:44 TheAceOfHearts: when'd you get promoted to oracle, caitp :p? sounds fancy
11:45 caitp-: the oracle isn't necessarily me at this hour
11:45 caitp-: it might be someone else
11:45 caitp-: the channel is collectively an oracle of pseudo-knowledge
11:45 TheAceOfHearts: haha
11:45 (Wizek) caitp-: I am trying to implement a multi-transclude directive, and it works suprisingly well, but there is an issue: Everything inside gets $compiled as many times as there are "holes" defined (named transclusion targets). Is there a way to tell $transclude to only compile the cloned nodes once?
11:45 TheAceOfHearts: pseudo-knowledge describes all of my knowledge quite well
11:46 caitp-: wizek first of all, what do you mean by multi-transclude
11:46 TheAceOfHearts: you can have multiple transcludes?
11:46 caitp-: are you saying that you're doing something like polymer's <content> selector stuff?
11:50 (Wizek) caitp-: You know, if you use ng-transclude you have one origin and one target. The origin is whatever was inside the directive before it got replaced by the template. Target is wherever the template has ng-transclude attribute. Nice and simple. Now, I want to be able to have named targets: <div ng-transclude="foo"></div> and named origins: `<foo>t'clude me please</foo>`, and
11:50 (Wizek) the right content should find the right hole.
11:51 (Wizek) TheAceOfHearts: You can if I can make it work well. :)
11:52 (Wizek) caitp-: looking into polymer to say if they are similar.
11:52 TheAceOfHearts: I'd probably find it too confusing; if I ever want to have multiple transclusions I'd just make a handful of directives that work together
11:52 TheAceOfHearts: and hopefully keep code smaller
11:54 (Wizek) TheAceOfHearts: I can give you a usecase: <modal><header>hello</header><footer><button ng-click="$closeModal()">ok</buton></footer></modal>
11:55 TheAceOfHearts: right, that's 3 directives
11:55 (Wizek) TheAceOfHearts: The modal in question has a third hole: content, but that is unused in this instance.
11:55 (Wizek) not exactly
11:55 TheAceOfHearts: you can achieve the same thing with a few directives :p
11:56 (Wizek) because, you see, named holes can also contain default content
11:56 (Wizek) so, if you don't provide <footer>, it will defult to a button saying ok.
11:56 caitp-: are you working on angularjs or angulardart?
11:56 (Wizek) caitp-: js
11:56 caitp-: because this sounds very dart/polymer-y
11:56 TheAceOfHearts: I guess
11:56 caitp-: unless you've implemented multi-transclusion on your own
11:57 caitp-: which you certainly might have
11:57 TheAceOfHearts: I'm too lazy for this
11:59 (Wizek) caitp-: Can/Should polymer be used in conjunction with angular?
12:00 caitp-: it can be, whether it should depends on if you get anything out of it
12:04 (Wizek) caitp-: Hmm, I don't know much about polymer, but this feature should be possible to implement using angular only, right? I mean, the directive I wrote does everything I described, it just has this double-compile problem. Any ideas how to prevent that?
12:05 caitp-: all you need to do is decide where to transclude stuff to in your transclude before-insertion-thing
12:05 caitp-: you can do that anywhere
12:06 caitp-: you could say forEach(nodes, function(node) { if (node.matches(".header") header.append(node) else if (node.matches(".footer")) footer.append(node) else content.append(node)
12:11 (Wizek) caitp-: Well, yes, inside the $transclude function I have access to the tcOriginNodes, but not outside (or not without calling it first). So that means I need to call it at leas once to get them. Also, I noticed that the tcOriginNodes get compiled as many times as I call $transclude, so that means I can call it at most once. Btw, do you know why $transclude even lends itself
12:11 (Wizek) for calling it multiple times? Is there some usecase for that?
12:11 (Wizek) it seems like compiling the same set of node with different scopes is not really a good idea (well, maybe outside ng-repeat)
12:11 caitp-: you don't want to call the transclude function more than once, generally
12:17 (Wizek) caitp-: Thanks for confirming that suspicion of mine. :) Or, as an improvement on ng code, would it make sense to make it only compile once however many times you call it? And it would just return the cloned elements? I guess not, because it always returns them compiled, and if they are cloned they need to get compiled for all callse. Except, if there was an option for advanced
12:17 (Wizek) transclusion, meaning that it gives you the tcOriginNodes without compiling, but then of course the requesting code will have to take care of it. That would actually solve my problem. Is there a way to request/access the raw tcOriginNodes?
12:19 caitp-: transclude() is going to compile the transcluded nodes, every time you call it
12:20 caitp-: you don't have to attach specific nodes to a given element, though
12:20 caitp-: that's your call
12:20 caitp-: my sentences are gonna stop making sense pretty soon because the compiler is confusing and i've been up all night
12:21 (Wizek) caitp-: I hope I can understand how it works before that happens. :)
12:23 (Wizek) caitp-: Is there someone else around now that knows about the internals at least as much as you do? 
12:23 caitp-: at this hour? idk
12:23 caitp-: i don't totally understand what you're confused about though
12:23 caitp-: by which I mean, I'm not sure what you're asking
12:24 TheAceOfHearts: pretty niche knowledge
12:24 caitp-: you have a templatel ike this
12:24 caitp-: <my-directive><p>content for directive</p><p>more content</p></my-directive>
12:24 caitp-: transclude will compile those <p> elements
12:24 caitp-: they don't get added to the DOM until you do that manually
12:25 caitp-: in the attach callback, you can choose where you want to attach the <p> elements in your directive template
12:25 caitp-: if you don't transclude them, those elements never get compiled
12:26 caitp-: but I don't really know what you're asking
12:26 caitp-: "is there a way to request/access the raw tcOriginNodes"? << what do you mean
12:27 caitp-: Idk what "tcOriginNodes" are, I assume you mean the nodes that get passed to your attach function
12:27 (Wizek) correct.
12:27 caitp-: and what do you mean by "raw"
12:27 (Wizek) not-yet-compiled
12:27 caitp-: they aren't compiled
12:28 caitp-: like, just to begin with, they aren't compiled
12:28 (Wizek) hmm
12:31 (Wizek) caitp-: let's suppose you have something like this: <my-directive><p ng-init="alert(1)">content for directive</p><p ng-init="alert(2)">more content</p></my-directive> and your $transclude function always only transcludes the first p tag, and discards the second one.
12:32 (Wizek) if I understand $transclude correctly the result will be that both alerts get called
12:32 (Wizek) even though one of them was discarded
12:32 (Wizek) Do I understand it correctly? (throwing together a plunker)
12:35 caitp-: the pipeline is like, compile -> attach -> prelink -> children get processed -> post link
12:36 caitp-: the entire collection of elements that get passed to the attach function will get "compiled", but if you remove items from the collection, they shouldn't get linked
12:36 caitp-: far as I'm aware
12:43 (Wizek) caitp-: http://jsbin.com/yoqowaha/2/edit
12:44 (Wizek) console says 1 and 2 for me
12:44 (Wizek) caitp-: do you know how to make it say only 1?
12:45 caitp-: i hate jsbin
12:45 caitp-: why is it so bad ;(
12:45 caitp-: no "fork" button
12:45 (Wizek) caitp-: there is
12:45 caitp-: anyways, save the filtered nodes
12:45 caitp-: to a variable
12:45 caitp-: and return that variable from the function
12:46 (Wizek) caitp-: top-left corner menu > clone (cmd+shift+s)
12:46 caitp-: oh, it still links both
12:46 caitp-: well then, I guess you're out of luck ;)
12:46 (Wizek) caitp-: suprising, isn't it? Or is it a bug because it is not supposed to link both?
12:53 (Wizek) caitp-: Thinking about it, I am getting more confident to say: only appended nodes should be linked. There is no use in linking detached ones, right? I guess, I'll open an issue about it. Thank you very much for the tango. :)
12:53 caitp-: well, unfortunately that's kind of a tricky one to fix
12:54 (Wizek) caitp-: why?
12:56 caitp-: look at the compiler and see for yourself
12:57 (Wizek) caitp-: I have been doing that the past 3 days, I might as well continue. :)
12:58 (Wizek) caitp-: Even if it is techincally challenging, I think I will open the issue to get validation by others saying thet it is indeed something to be fixed.
12:59 caitp-: go nuts, but I wouldn't put too much hope on it getting fixed
12:59 caitp-: maybe I can look at it later in the week
13:01 (Wizek) caitp-: That would be very nice. I am also looking into it. My main deterrent so far has been the lack of knowladge about the internals, but I guess that will come with time. Would you be kind enogh to share your findings with me even if you can't find a complete solution?
13:02 caitp-: well if I work on it I'll come up with something, but our focus isn't really on the 1.3 stuff right now
13:13 (Wizek) caitp-: https://github.com/angular/angular.js/issues/7270 . Thank you once more, and get some rest. :)

[Wizek]

Actually, changing angular's compile like this in might enable a solution:

--- angular/angular.js Sun Apr 27 16:32:20 2014
+++ angular/angular.js Sun Apr 27 16:32:45 2014
@@ -5887,8 +5887,13 @@
           }
         }

-        if (cloneConnectFn) cloneConnectFn($linkNode, scope);
-        if (compositeLinkFn) compositeLinkFn(scope, $linkNode, $linkNode);
+        var overrides = {};
+        if (cloneConnectFn) {
+          angular.extend(overrides, cloneConnectFn($linkNode, scope));
+        }
+        if (compositeLinkFn && !overrides.alreadyLinked) {
+          compositeLinkFn(scope, $linkNode, $linkNode);
+        }
         return $linkNode;
       };
     }

This would enable the user to compile/link the way they want to.

$transclude(function(tcOrigin, tcParentScope) {
  /*
  ...
  */
  tcTarget.empty().append($compile(candidates)(tcScope));

  return {
    alreadyLinked: true
  };
});

[Wizek]

@Wizek well, the question is if #7270 is really a bug

@lgalfaso For the sake of argument, let's suppose this is not a bug. Can you tell me why linking up detached nodes is a good idea? And even if you come up with a corner case, do you think that the users should be barred from controlling what gets linked up even when they want to have a finer level of control?

[lgalfaso]

@Wizek I am not saying that linking a detached node is a good/bad idea, but the current contract is that the inner function will take care of the node always. How this is done, it is another problem. Now, I do see some value in the end goal, but we have to be sure that this does not cause any regressions on the way existing users are already using and abusing this contract