Broken ngRepeat directive in angular.js 1.2

[kamilkp]

Hi,
I am currently using angular.js 1.2.0 in my projects and I stumbled across some problems with ngRepeat. The problem occurs, when i move some of the ngRepeat's rendered elements into another place in DOM. Sample app where the problem occurs can be found here: http://kamilkp.co.nf/brokenNgRepeat/

You need to:

1. click "reorder first two items" button
2. click "replace list model" button.

First action takes the second ngRepeated div detaches it and places it as the first element in the containter (using simple jQuery detach and prependTo). The second action replaces the ngRepeat "items" array with a new array. This causes a "cannot call method insertBefore of undefined" error.

As opposed to angular 1.2.0 ngRepeat directive the same app (and same scenario) works as expected with angular 1.1.4 (http://kamilkp.co.nf/workingNgRepeat/)

You can easily "x-ray" both apps with Chrome dev tools and they should be pretty straightforward.

This issue is super annoying and it exists also in 1.2.0 rc3 and rc2. Up until now i've been copying old angular ngRepeat directive from 1.1.4 version along with $animator and $animation services to work. But now that i have some awesome animations with new ngAnimate API i'm stuck ;/

The only way to make the new ngRepeat directive work is to manually move the html comment node "end ngRepeat" along with the element being moved (i see that now that comment is present after each ngRepeated element, not after last element as it was in the old version). But doing that is just sad ;(

P.S. in my projects i need this kind of behaviour to work because i implement some complex animations which consist of e.g. moving elements beetwen their corresponding arrays.

[matsko]

There are a couple of open PRs related to insertion errors with $animate (since ngRepeat uses $animate internally). I'll be getting to this tomorrow and it should make it in for 1.2.2.

[matsko]

Sorry for the delay on this, but it will need to wait for next week's 1.2.3. There are three other issues which are very similar and I would like to have them all fixed in the same patch.

[matsko]

Alright so I finally got at this. Turns out you're moving around the elements using your own DOM code. This basically makes the repeater results and the DOM results out of sync and ngRepeat uses multiple comment anchors ontop of each list element to keep track of where they are. Then when ngAnimate runs, it doesn't know if it is dealing with a comment node or the element node. As a best practice, you don't want to do your own DOM stuff. Use a orderBy in your repeater and provide a scope function to handle the sorting (look at http://stackoverflow.com/questions/12040395/angularjs-custom-sort-function-in-ng-repeat#answer-12041694). If you really wanted do your own DOM sorting yourself then you might be able to get this to work by moving the comments around too, but don't do that :P

Either way, this is not a bug with ngAnimate or AngularJS.

[kamilkp]

Thank you for your reply, but look, if i needed just this kind of simple sorting, I wouldn't bother writing an issue on github about it. In a post scriptum i wrote that i need this kind of DOM manipulations to work because i implement a custom animation of moving an element from one container (some array of elements) to another. The two containers are aligned vertically, and when a user clicks the "move" button, the clicked element is detached from the layout with position: absolute, an in it's place i put a placeholder element of its exact height and width, then in the destination container i put a placeholder element of hight = 0, and the same width. Then i animate the height of the first placeholder from it's initial height to 0, and in the same time i animate the height of the second placeholder from 0 to the height of the element being moved. This gives the user an impression that the elements between the source and destination place are moving downwards/upwards whereas the height of the element wrapping both containers stays the same. During the placeholders' height animation i also move the absolutely positioned element upwards/downwards in the same time transforming it's scale and rotation transform properties. How do you suppouse i manage to do this without my own DOM manipulation?

By the way, this "new" ngRepeat will break (throw this "cannot call method insertBefore of undefined" error), even using the standard jQuery-ui sortable (http://jqueryui.com/sortable/) or angular-ui sortable (https://github.com/angular-ui/ui-sortable) or angular-ui-multi-sortable library (https://github.com/mostr/angular-ui-multi-sortable). The latter library is actually what i also use in my projects to move elements between different arrays, and this is where i first stumbled across this strange behaviour of ngRepeat. And yes, I know that moving the comment tags along with the elements that they correspond to will make ngRepeat work, i figured it out earlier, and i had to write this peace of code a the beginning multiSortable.js "_update" method make it work:

       var counter,
            origSubsetElement = $('[model-subset="' + self.data.origSubset + '"]')[0],
            childNodesOfOrigSubset = origSubsetElement.childNodes,
            destSubsetElement = $('[model-subset="' + self.data.destSubset + '"]')[0],
            childNodesOfDestSubset = destSubsetElement.childNodes,
            origCommentIndex = -1,
            destCommentIndex = -1,
            commentElement,
            childNodesOfOrigSubsetLength = childNodesOfOrigSubset.length,
            childNodesOfDestSubsetLength = childNodesOfDestSubset.length;

        counter = self.data.origPosition;
        while(counter >= 0 && origCommentIndex < childNodesOfOrigSubsetLength - 1){
          origCommentIndex++;
          if(childNodesOfOrigSubset[origCommentIndex].nodeType === 8 &&
            /end ngRepeat/.test(childNodesOfOrigSubset[origCommentIndex].data))
            counter--;
        }
        if(counter < self.data.origPosition){ // angularJS 1.2
          counter = self.data.destPosition;
          while(counter >= 0 && destCommentIndex < childNodesOfDestSubsetLength - 1){
            destCommentIndex++;
            if(childNodesOfDestSubset[destCommentIndex].nodeType === 1)
              counter--;
          }

          commentElement = childNodesOfOrigSubset[origCommentIndex];
          // childNodesOfOrigSubset[origCommentIndex].remove(); // remove works only in chrome
          childNodesOfOrigSubset[origCommentIndex].parentNode.removeChild(childNodesOfOrigSubset[origCommentIndex]);
          destSubsetElement.insertBefore(commentElement, childNodesOfDestSubset[destCommentIndex + 1]);
        }

I personally think that it's not really convenient that we now need to write that kind of code to achieve simple ui sortable behaviour. Why did you have to write a new way that ngRepeat works? The old one was perfectly fine, and it didn't require this kind of struggling on developer's part. I suppouse this new way was written in order to make ng-repeat-start and ng-repeat-end possible, but what is the benefit of that? Without that i could just wrap the elements i wanted to repeat in a div and apply ng-repeat directive on that wrapping div. It's even simpler to understand when reading someone else's code.

[kamilkp]

I have written a module to obtain a sortable behaviour with no jQuery nor jQueryUI needed whatsoever. And it also doesn't break the ngRepeat inner logic. The module can be found here and a demo here