ngRepeat and ngIf introduces comment nodes into animations ($animate)

[chrisirhc]

Update: #5570 attempts to introduce a 4th option, to call performAnimation on each node.

Plunker that demonstrates the issue (see error in console after clicking the button): http://plnkr.co/edit/ncQSgi4Y44P3ZyYM04Wv?p=preview

Currently, ngRepeat and ngIf directives add a comment node after the inserted content. As such, the enter animation contains this comment node.
This violate the Principle of Least Astonishment as a user of the directive would only expect the animation to contain the element(s) that they added.

I see three possibilities from here (note these possibilities aren't mutually exclusive):

1. Modify $animate documentation to indicate that the enter animation is called with contents rather than element.
   At least users will know to expect that it will be called with non-elements. This is similar to the distinction between jQuery's contents() vs children().
2. ngRepeat and ngIf shouldn't call animation on those additional comment nodes.
   No animation can occur on those nodes, anyway.
3. Add an animation method that explicitly supports multiple nodes/elements say $animate.multiEnter, and this method will call $animate.enter with one element at a time and only fire the done callback of the multiEnter when all have elements have animated.
   I noticed that the $animate.enter is only animating the whole ngRepeat block with the first element node that it finds. This means that if there were multiple elements with different elements in the ngRepeat, the animations won't run as expected.
   Similar to option 1, the multiEnter parameters will need to explicitly indicate that the first argument is contents rather than element.

Note: When going for option 1 or 3, it's a good idea to add a filter method in jqLite that supports only tagNames, so users can simply do contents.filter('*') to filter the contents for element nodes. This way, users can apply css method without worry for an exception thrown when calling the css method on a comment node.

In my exploration into this issue, I noticed that ngRepeat can be written so that it doesn't use comment nodes (I'm aware of #3104). ngRepeat can actually read up to the next block and decide that everything between is part of the current block. However, this adds unneeded complexity to the code and isn't a good solution, I feel, as it assumes that the first element of the next block is the start of the next block (which may not be the case if a directive decides to change that by adding an element before it).

I can work on a PR but I just wanted to put this issue up so that someone can look at it and perhaps tell me if there is something in the works that will nullify any of these solutions (with perhaps a better solution).

Related to #4786 , #4157 .

[matsko]

@chrisirhc thank you for the issue and the PR. Before I start looking into this. Does #5570 take care of this issue?

[chrisirhc]

Yes it does take care of this issue. Sorry, should've been clearer in the PR.

[matsko]

@chrisirhc Sorry I just found that this issue was open from long ago. Looks like this was fixed some time back. Sorry for not looking into this. Thank you for the work you put into it.