Cloned nodes store ref to original on originalNode, inheritedData checks...

[colinkahn]

... if element isClone and if so checks data on originalNode

For issue #2533

[petebacondarwin]

This is also related to this bug: #1805.

@colinkahn , I think a better solution is to make a copy of the data attached to a node when we clone it. That way, we don't have to try to track whether nodes are cloned and more easily handles the situation where the original node gets deleted.

[petebacondarwin]

Moreover, I think this version is dangerous in that it allows your cloned node to access controllers that are inherited by the original node, which doesn't make sense. If you move a node to a new point in the DOM then its inheritedData should change accordingly.

[petebacondarwin]

As of 1.4 jQuery provides a parameter to cloneWithDataAndEvents. I guess we need something like this. I am not sure about the events though since they would need to be attached to a suitable scope.
We probably should only copy across angular specific data too?

[petebacondarwin]

This is further complicated by the fact that if you are cloning a node and creating a new scope for it then you need to wire up the controller with this new scope, rather than use the original controller (or even a clone of it).

From looking at the code, the following Angular specific data is added to nodes:

• $injector - this is a singleton and currently only attached to the root of the application at bootstrap time. So should probably not be copied or referenced.
• $scope - this is attached to a node by the compiler when it determines that a new scope is needed at this node. This only happens at two places in the compiler, publicLinkFn and compositeLinkFn. In each case, I think that a new scope is going to be attached shortly after cloning if needed so this should not be copied or referenced.
• $binding - this is applied at compile time by ngBind... suite of directives. At the moment, these have a lower priority than ng-repeat so are not affected. Also, for ng-include and ng-view, any binding on the element will be removed when the template is rendered so again has no impact. So I would again suggest that there is no need to copy or reference $binding. There may be other directives, that I have not noticed or more frustratingly, someone could create a directive with priority 0 that terminates and clones but expects an ng-bind on the element to work on the clone. For the time being we should leave this as is.
• $[controllerName]Controller - this is applied only at compile time when a controller is defined for a directive. This happens before subsequent lower priority directives get triggered. If the lower priority directives then clone the element, the controller does not appear on the clone and so causes unexpected behaviour on the clones. This is likely to happen in many cases, using ng-repeat, but also where directives replace their elements with templates. When the node is cloned we need a new copy of the controller to appear on the clone, but this copy needs to be bound to the correct scope of the cloned element.

From looking at above, it seems that actually the $[controllerName]Controller is actually a special case that should be dealt with by the compiler when it is cloning nodes. It should instantiate a new controller with using name of the controller from the original element using the scope that is attached to the clone.

[petebacondarwin]

Just to clarify what scope should be bound to a cloned node's controller. I think that if the element has been cloned for transclusion then its scope should be the transclusion scope. Is that correct?

[petebacondarwin]

By the way, this is all really about transclusion, not any particular directive.

[petebacondarwin]

@colinkahn & @IgorMinar - not made much progress with this as it is highly convoluted inside the compiler. Any further ideas?

[colinkahn]

@petebacondarwin thanks for taking a look and providing these details. Overall this definitely raises a few questions for me.

Regarding the point about inheriting data when a cloned element changes parents, I agree that this makes my solution less than optimal. My current solution to work around this is similar to what you had suggested, which is basically copying the data from the original to the cloned element (in coffeescript):

copyData =  (el1, el2) ->
    for name, value of el1.data()
      el2.data name, value unless name is "$scope" # don't overwrite our $scope

I guess where it gets confusing is when you start mentioning cloned controllers. Currently, if I have a directive that behaves like ngRepeat but has a controller and I use my copyData on it, then all the clones will share the original controller (essentially the controller for the "group" of stamped out clones). IMO this makes sense, having each cloned element get its own new copy of the directives controller means losing the connection between all of them. If clone-by-clone logic needs to happen then the clone can access methods on the "group" controller, or do something similar to ngSwitch, registering themselves on that controller to be used later.

I think that would greatly simplify solving the problem, but of course, this is just my opinion on how all of this should work.

[mhevery]

I have commented on the issue. Could you better explain what the issue is and what you think should happen. As it stands right now having a controller on ngRepeat makes no sense since it is unclear what should happen if controller goes to zero and with which element should the controller be associated with.

[petebacondarwin]

Let's close this PR and work on the issue toward a final solution.